-
Notifications
You must be signed in to change notification settings - Fork 3
Matrix 0-indexed #1467
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
Matrix 0-indexed #1467
Conversation
|
Thanks @jacobpake -- |
Thanks - I struggled to understand the |
|
@rolyp I made the changes to I change |
rolyp
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.
Looks great, just some minor suggestions re. the new exprBwd case for [e1 .. e2].
This PR makes matrices 0-indexed (for consistency with lists), updating the internal and library functions.
The above produces a
nxmmatrix as before, butiandjhere will be0..n-1and0..m-1rather than1..nand1..m(is this what we want?).dimswill return the size as before.Internals diff
Library diff
Most tests were straightforward to update but there are two notable cases that could use another set of eyes:
array.fld- the values in the matrix are initialised from the indices, so the matrix (and test output) now contains different values (is this right?)compute-dtw.fld- output is different, either similar to above or I may have made a mistake intest/fluid/lib/dtw.fld(this was a bit delicate)Fixes #790