-
-
Notifications
You must be signed in to change notification settings - Fork 359
V thomas #634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V thomas #634
Conversation
| d[i] -= c[i] * d[i+1] | ||
| } | ||
|
|
||
| return d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning something that you've also mutated as an input argument isn't good no matter the language. You should return d, remove mut for c and d, and make copies of those two input arrays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are of course right. I changed it.
Thanks 👍
| println("[2.0 3.0 5.0][y] = [5.0]") | ||
| println("[0.0 3.0 6.0][z] = [3.0]") | ||
| println("has the solution:") | ||
| solve := thomas(a, b, mut c, mut d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| solve := thomas(a, b, mut c, mut d) | |
| solution := thomas(a, b, mut c, mut d) |
is more descriptive to me.
|
Can you revert your changes to |
| @@ -0,0 +1,35 @@ | |||
| fn thomas(a []f32, b []f32, c []f32, d []f32) []f32 { | |||
| mut new_c := c | |||
| mut new_d := d | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't actually making a copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right. this just creates a new pointer, pointing to the old array.
this defeats the whole purpose of mut, as the current implementation of V mutates the original array as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes. Filed vlang/v#2548.
In the meantime, can you do it "by hand"? Write a function that, given an array, makes a new one, iterates over the old one pushing each item into the new one, then returns the new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meaning, while I acknowledge there is a compiler bug, we should implement the workaround anyway (at least for now) in the interest of safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked Alex (the main dev) already about it. he knows and is currently reworking the memory stuff. after that is done this bug should be gone as well
This reverts commit bf91a89.
berquist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take your word on it about the bug, plus the linked issue will let me know to re-test this. Everything else looks good.
thomas algorithm in vlang