added thomas alg in nim #399

c252 · 2018-09-29T14:56:30Z

implemented thomas algorithm in nim

jiegillet

The algorithm looks fine, I just have a couple of small requests.

jiegillet · 2018-10-03T01:22:06Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+
+echo "The system,"
+echo "[1.0 4.0 0.0][x] = [7.0]"
+echo "[2.0 3.0 5.0][x] = [5.0]"


I would write [y] and [z] instead fo [x] everywhere

yeah that was a copy paste fail on my part.

Don't forget to fix this :)

jiegillet · 2018-10-03T01:23:18Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

@@ -0,0 +1,34 @@
+proc thomas_algorithm(a,b,c,d: var array[3,float]) = 


It's a personal bias of mine, but I prefer functions that don't modify the input. Is there an easy way to return the modified d without changing the original?

I can do that, it might look less neat though. Because I would have to create new variables in the thomas alg proc and assign them to the inputs.

You only need to copy c and d though, it's only a couple more lines... There should be some kind of copy function that does it.

On that very same line: array[3,float].
Having a 3 there is a big no no!

is that because the size shouldn't be hard coded?

jiegillet

Next round :)

jiegillet · 2018-10-03T13:29:29Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

@@ -0,0 +1,34 @@
+proc thomas_algorithm(a,b,c,d: var array[3,float]) = 


You only need to copy c and d though, it's only a couple more lines... There should be some kind of copy function that does it.

jiegillet · 2018-10-03T13:30:23Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

@@ -0,0 +1,34 @@
+proc thomas_algorithm(a,b,c,d: var array[3,float]) = 


On that very same line: array[3,float].
Having a 3 there is a big no no!

jiegillet · 2018-10-03T13:31:25Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+var x: array[3,float] = [0.0,2.0,3.0]
+var y: array[3,float] = [1.0,3.0,6.0]
+var z: array[3,float] = [4.0,5.0,0.0]
+var w: array[3,float] = [7.0,5.0,3.0]


If you make your thomas_algorithm pure, then you can change the var to const.

c252 · 2018-10-03T20:02:16Z

I might have messed something up with git but if this looks good, then I will change the import lines.

jiegillet

This is also not a great solution. I apologize for not knowing nim, I cannot really help you figure out the answer, but there must be a cleaner way.

jiegillet · 2018-10-03T23:56:08Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+
+const n: int = len(w)
+
+proc thomas_algorithm(a,b,c_in,d_in: array[n,float]): array[n,float] = 


That solution is also not satisfying, you shouldn't rely on a global variable outside of the function, this is basically hardcoding in disguise.

jiegillet · 2018-10-03T23:56:54Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

@@ -0,0 +1,40 @@
+const x: array[3,float] = [0.0,2.0,3.0]


It's more readable if these are defined after the function. I suspect that you had to do this for the function to know n but that's not the way you should do it.

I could pass the length of the arrays to the thomas_algorithm function. Would that be okay?

Yes, that would be fine

jiegillet · 2018-10-03T23:57:50Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+    #const n: int = len(d1)
+
+    var c: array[n,float] = c_in
+    var d: array[n,float] = d_in


This is quite clean I think :)

jiegillet · 2018-10-03T23:59:49Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+
+echo "The system,"
+echo "[1.0 4.0 0.0][x] = [7.0]"
+echo "[2.0 3.0 5.0][x] = [5.0]"


Don't forget to fix this :)

berquist

We talked in chat about the solution, but a few other things:

Please always put spaces around binary operators (a[i-1] vs. a[i - 1]). Similarly, add a space after a comma ([1.0,3.0,6.0] vs. [1.0, 3.0, 6.0]).
In the style guide, they say indentation should be two spaces. Can you add these lines to the .editorconfig file at the project root?

# Nim
[*.nim]
indent_style = space
indent_size = 2

jiegillet · 2018-10-05T12:03:57Z

The precise use of seq is beyond my understanding, the rest looks fine to me now. @berquist the final word is all yours.

c252 · 2018-10-05T12:42:41Z

Cool, I will change the import lines when I get the ok from berquist

berquist · 2018-10-05T14:10:30Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+
+const soln: seq[float] = thomas_algorithm(x,y,z,w)
+
+for i in 0..2:


Can you calculate this range based on the number of equations again, rather than have it be hard-coded?

berquist · 2018-10-05T14:11:17Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+  c[0] /= b[0]
+  d[0] /= b[0]
+
+    for i in 1..n-1:


There is accidental indentation from here to the end of the procedure. Also n-1 -> n - 1.

berquist · 2018-10-05T14:12:07Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+      c[i] *= scale
+      d[i] = (d[i] - a[i] * d[i - 1]) * scale
+
+    for i in countdown(n-2,0):


(n-2,0) -> (n - 2, 0)

berquist · 2018-10-05T14:12:30Z

contents/thomas_algorithm/code/nim/thomas_algorithm.nim

+
+echo "has the solution:"
+
+const soln: seq[float] = thomas_algorithm(x,y,z,w)


(x,y,z,w) -> (x, y, z, w)

c252 · 2018-10-05T15:54:47Z

I will fix these when I get home from school, I did fix the indentation previously but I must have messed something up with git.

c252 · 2018-10-05T20:04:30Z

I made the formatting changes that Berquist requested and the array print loop is not hard coded any more.

berquist · 2018-10-05T20:09:08Z

I can't render it right now because I'm at work, but

there is a conflict in the Markdown file, and
you accidentally committed the generated *.c and *.o files.

c252 · 2018-10-05T20:18:50Z

Should be good now.

berquist · 2018-10-05T23:48:48Z

I am glad this was figured out. Thank you!

implemented thomas algorithm in nim

added thomas alg in nim

818fd32

Gathros added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label Sep 30, 2018

leios added the Hacktoberfest The label for all Hacktoberfest related things! label Oct 1, 2018

jiegillet requested changes Oct 3, 2018

View reviewed changes

jiegillet self-assigned this Oct 3, 2018

Update thomas_algorithm.nim

a06341d

jiegillet requested changes Oct 3, 2018

View reviewed changes

c252 added 2 commits October 3, 2018 15:53

Update thomas_algorithm.nim

b5bdce3

Update thomas_algorithm.nim

887c35a

jiegillet requested changes Oct 4, 2018

View reviewed changes

berquist requested changes Oct 4, 2018

View reviewed changes

Update thomas_algorithm.nim

755cb2c

jiegillet approved these changes Oct 5, 2018

View reviewed changes

berquist requested changes Oct 5, 2018

View reviewed changes

made final edits

d0a22bf

c252 added 2 commits October 5, 2018 16:15

deleted .0 and .c files

578ca13

Merge branch 'master' into thomas_algorithm_in_nim

1f80db8

fixed

c06e13f

berquist approved these changes Oct 5, 2018

View reviewed changes

berquist merged commit 93c1cd9 into algorithm-archivists:master Oct 5, 2018

kenpower pushed a commit to kenpower/algorithm-archive that referenced this pull request Oct 22, 2018

added thomas alg in nim (algorithm-archivists#399)

9f03784

implemented thomas algorithm in nim

		@@ -0,0 +1,34 @@
		proc thomas_algorithm(a,b,c,d: var array[3,float]) =


		const n: int = len(w)

		proc thomas_algorithm(a,b,c_in,d_in: array[n,float]): array[n,float] =


		const soln: seq[float] = thomas_algorithm(x,y,z,w)

		for i in 0..2:


		echo "has the solution:"

		const soln: seq[float] = thomas_algorithm(x,y,z,w)

Uh oh!

added thomas alg in nim #399

added thomas alg in nim #399

Uh oh!

Conversation

c252 commented Sep 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jiegillet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiegillet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c252 commented Oct 3, 2018

Uh oh!

jiegillet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berquist left a comment

Choose a reason for hiding this comment

Uh oh!

jiegillet commented Oct 5, 2018

Uh oh!

c252 commented Oct 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c252 commented Oct 5, 2018

Uh oh!

c252 commented Oct 5, 2018

Uh oh!

berquist commented Oct 5, 2018

Uh oh!

c252 commented Oct 5, 2018

Uh oh!

berquist commented Oct 5, 2018

Uh oh!

Uh oh!

c252 commented Sep 29, 2018 •

edited

Loading