# Tag Info

## Hot answers tagged code-review

22

This is a simplification of your solution: LanguageExtendedFullDefinition[new] = LanguageExtendedFullDefinition[old] /. HoldPattern[old] :> new I believe Language`ExtendedFullDefinition is used in transferring definitions between the main kernel and subkernels. Also note the HoldPattern on the LHS of the rule which ensures that OwnValues will ...

17

Here is a hybrid recursive/StringReplaceList method. It builds a tree representing all possible splits. Now with a massive speed improvement thanks to Rojo's brilliance. elements = ToLowerCase @ Array[ElementData[#, "Symbol"] &, 112]; altelem = Alternatives @@ elements; f1[""] = Sequence[]; f1[s_String] := Block[{f1}, StringReplaceList[s, ...

13

Here is a fairly simple approach using only higher level functions. First, note that StringCases does almost all the work for you. István mentioned it in passing, but it is more powerful than that. It has an Overlap option that you can set to True to get all possible decompositions in one go: elements = Table[ElementData[i, "Symbol"], {i, 112}]; ...

12

Some really simple partial answers using the string patternmatcher: elements = ToLowerCase /@ Select[Table[ElementData[i, "Symbol"], {i, Length@ElementData[]}], StringLength[#] < 3 &]; StringReplace["archbishop", # -> {#} & /@ elements] /. StringExpression -> Join StringReplace["titanic", # -> {#} & /@ elements] /. ...

11

The following seems a little more elegant. data = Import["http://www.massey.ac.nz/~pscowper/ts/cbe.dat"]; ts = TemporalData[data[[2 ;; -1, 1]], {"1958", Automatic, "Month"}]; DateListPlot[ts["Path"]] TemporalData can also store multiple paths. ts2= TemporalData[Transpose[data[[2 ;; -1]]], {"1958", Automatic, "Month"}]; DateListPlot[ts2["Paths"]]

11

One can also go about this using integer linear programming, with an array of 0-1 variables indexed by vertices and colors. Here is one encoding of that approach. constrainedColorings2[graph[vertices_, nbrhds_], colors_List, start_List, v_] := Module[ {unassigned, nv = Length[vertices], nc = Length[colors], vars, fvars, c1, c2, c3, c4, pos1, pos2, ...

10

I compiled your exact algorithm, and it seems to work OK. I get 15 times speed up when using WVM as target, and 60 times when using C (CompilationTarget->"C"). The output is: {{e, vir, 0}, f} test = Compile[{{nparticle, _Integer, 0}, {rho, _Real, 0}, {rc, _Real, 0}, {r, _Real, 2}, {f, _Real, 2}}, Module[{vir = 0., e = 0., dr = {0., 0., 0.}, ...

10

First, I notice that you are using Real numbers such as 1. and 2. for Part indexes. While this works it would be better to use Integer indexes, 1 and 2. Your use of PregaoMC and then Table, etc., is highly inefficient. Part and Span will be better. Observe: Table[PregaoMC[x], {x, 2, n}] === CompleteMatrix[[2 ;; n, 1]] True n = 359835; ...

9

I can't find the actual code in your linked data file, but it may be worth posting my own solution for a 2D Poisson problem here. It is copied from my web page. I'm using a maximum of 100000 iterations by default. From your description, it sounds as if you could try to re-write your loops using constructs such as Fold, Nest or - as I do below - FixedPoint. ...

9

First I want to say, as you mentioned in your comment that your ultimate goal is to to do it for nMax over 100, I suggest you first symbolicly calculate the correlation of the following function, treating $r_n$ ($n=-s,-s+1,\dots,s$, and $s$ is nSteps for short) as variables as $x$: \xi(x,r_{-s},r_{-s+1},\dots,r_{s})=\sum _{n=-s}^{s} r_n\, ...

8

Minor improvement: RandomVariate accepts a second argument with the number of elements you want to create. So your assignments to tempX, etc, are equivalent to tempX = RandomVariate[NormalDistribution[0, Sqrt[EmitX]], 2]; tempY = RandomVariate[NormalDistribution[0, Sqrt[EmitY]], 2];

8

This response will keep the basic strategy of the exhibited code, but it will show some useful Mathematica notations that can shorten the code and emphasize its key features. See the bottom of this response for the code in textual form. First, we will use ⊕ to represent XOR, just like in the Wikipedia article. This operator has no built-in meaning in ...

8

Without reading Leonid's answer (which is probably better) I recommend something like this: fillDates[dates_] := Module[{f, all}, all = Part[DateList /@ (Range[##, 24*60^2] & @@ AbsoluteTime /@ dates[[{1, -1}, 1]]), All, {1, 2, 3}]; (f[#[[1]]] = #) & ~Scan~ dates; f[x_] := {x, 0}; f /@ all ] fillDates @ {{{2012, 1, 1}, 1}, {{2012, ...

8

Let me offer some advice. Do not start user symbols (variable names) with capital letters. These often may conflict with built-in functions. You have a full selection of Script, Gothic, Double-struck, and Greek characters to choose from to avoid these. Look at Palettes > Special Characters to see these, and hover the mouse pointer over any one of them ...

8

The replace is unnecessary here. When I see this right, then your only issue is, that you use the slot #3 inside the inner pure function. There, it would be belong semantically to the inner function but you want it to be the parameter of the outer one. This can be solved by using only for the inner function the (#...)& syntax. For the outer one, you use ...

7

Rather than applying ToExpression and then NumberQ, perhaps you could just use StringMatchQ with NumberString in the first place. StringMatchQ["assd??asd", NumberString] (* Out: False *) StringMatchQ["123.3", NumberString] (* True *)

7

As you expect, it's much faster to call RandomVariate fewer times. In fact, it'd be much much faster to call it only once at the beginning, generating large arrays and then operating on them: In[20]:= Bunch = {}; Timing[Do[ Particle = {RandomVariate[NormalDistribution[0, 1]], RandomVariate[NormalDistribution[0, 1]], ...

7

Here are two things you can do to speed up this code. 1. Do the convolution with symbolic y Because you have defined corr using SetDelayed, the table of Convolve expressions will be re-evaluated every time you evaluate corr[number]. The normalisation term with y=0 is causing a particular slow down, though I'm not sure why exactly. If you instead use Set ...

7

Your best bet is to remove the procedural programming Do, While and Append Statements, building lists with Append is not quick. Then embrace a functional programming approach on which Mathematica thrives. Making use of constructs like Transpose, Part, Nest, Map, Table and Fold. These are generally much faster and lead to eventually to less buggy code. If ...

7

You can implement list functionality with string operations, so it's straightforward to make the output of Mr.Wizard's elegant solution more readable while retaining the focus on string operations. Let's begin with a modified version of his solution (altelem is the same as before): f1[""] = ","; f1[s_String] := StringJoin[ StringReplaceList[s, ...

6

Here's a version that uses plain (not string) pattern matching and rule replacement, as well as recursion, to generate all decompositions. EDIT to add: This approach turns out to be suprisingly efficient. I made no attempt to optimize my solution, and it doesn't make use of the string-handling functions at all, and it's about half as fast as Mr.Wizard's ...

6

Update #2 Now significantly cleaner and more efficient. This uses an arbitrary maximum pattern length of 20. This should be sufficient for most words, but it could be raised at the loss of some performance. elements = ToLowerCase @ Array[ElementData[#, "Symbol"] &, 112]; altelem = ## | EndOfString & @@ elements; pat = StartOfString ~~ ## -> ...

6

In addition to the other answers, it's generally not very efficient to create a table using Append repeatedly. This is because every time you call p = Append[p, exp], Mathematica creates a new copy of the list. A more efficient way would be to use Table instead, i.e. Bunch = Table[ code; Particle, {i, 20000}];

6

If you want to translate Matlab code into Mathematica, my advice is - don't! As programming languages, the two are very different and an idiom that works well in one is unlikely to work well in the other. A fundamental theorem theorem in discrete dynamics states that if there's an attractive orbit, then it must attract at least one critical point. Thus, ...

5

Your definition is progressBar2 is essentially correct, but it is evaluating its arguments. This causes the symbol i to disappear from your example as evaluation replaces i with its value. The simplest fix is to SetAttributes[progressBar, HoldFirst]. However, we see that almost all Mathematica controls require us to specify explicit Dynamic wrappers for ...

5

I think this works. I'll slightly change the setup and also make a proper Module out of the main part. rc = 2.5; rc2 = rc^2; nn = 6; (* Was 3 in original post *) nparticle = 4*nn^3; rho = 0.4; L = (nparticle/rho)^(1./3.); hL = 0.5*L; rr3 = 1/(rc^3); ecut = 4.*(rr3^4 - rr3^2); LinCell = Round[(nparticle/4.)^(1./3.)]; lattconst = L/LinCell; r = ...

5

A slightly modified version of the function ginivalues from @SethChandler's Wolfram demonstration LorenzCurvesAndTheGiniCoefficient gives about 6000x speed-up. Define giniF[dt_List] := With[{sorted = Accumulate[Sort[dt]]}, N@Mean[2 MapThread[#1 - #2 &, {Range[1/Length[dt], 1, 1/Length[dt]], sorted/Last[sorted]}]]] Using the medals dataset: giniF ...

4

You could use FileNameSetter which seems a better choice for setting the path. If you have a DynamicWrapper containing your function call, it will be run every time the path is changed. When the user cancels, the path doesn't change, so it won't trigger anything or require special handling. Here I just use a dummy work function which prints the chosen files ...

4

Your approach looks fine to me, but here's an alternative using Outer and Thread: (Outer[Flatten[{##}] &, {#1}, #2, 1] & @@@ Thread[{l1, l2}]) ~Flatten~ 2 (* {{a1, a2, 1, 2, 3}, {a1, a2, 4, 5, 6}, {b1, b2, 10, 11, 12}, {b1, b2, 13, 14, 15}, {b1, b2, 16, 17, 18}, {c1, c2, 19, 20, 21}} *) You could also use Transpose[{l1,l2}] instead of thread ...

4

If : sol = DSolve[{D[x[t, th], {t, 2}] == -0.2*D[x[t, th], t]/2.30, Derivative[1, 0][x][0, th] == 10.8*Cos[th], x[0, th] == 0}, x[t, th], t]; then you can use the solution as : Plot3D[x[t, th] /. sol, {t, 0, 10}, {th, -Pi, Pi}] Some checks : x[t, th] /. First[sol] /. t -> 0 (* 0. *) Simplify[D[sol[[1, 1, 2]], t] /. t -> 0] (* 10.8 Cos[th] ...

Only top voted, non community-wiki answers of a minimum length are eligible