Skip to content
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

Document difference between ZeroMatrix & NewZeroMatrix; ZeroVector & NewZeroVector; IdentityMatrix & NewIdentityMatrix #4351

Closed
fingolfin opened this issue Mar 26, 2021 · 2 comments · Fixed by #5326
Labels
kind: discussion discussions, questions, requests for comments, and so on

Comments

@fingolfin
Copy link
Member

... or else clarify how they relate to each other (e.g. which is implemented in terms of which, and for which ones does one need to provide methods in order for a new MatrixObj type to work.

@fingolfin fingolfin added the kind: discussion discussions, questions, requests for comments, and so on label Mar 26, 2021
@fingolfin
Copy link
Member Author

OK, after sleeping on this and looking again, I now see why the two exist: NewZero* are constructors (taking a filter as first argument), the others are regular operations. OK, got it. So we probably want to keep both, but should point out the difference explicitly, and also say which is implemented in terms of which (if at all).

@fingolfin fingolfin changed the title Merge ZeroMatrix & NewZeroMatrix; ZeroVector & NewZeroVector; IdentityMatrix & NewIdentityMatrix? Document difference between ZeroMatrix & NewZeroMatrix; ZeroVector & NewZeroVector; IdentityMatrix & NewIdentityMatrix Mar 29, 2021
@fingolfin
Copy link
Member Author

In PR #4366 I am removing most ZeroMatrix/IdentityMatrix methods, keeping just one for each set of argument types; they all dispatch to NewZeroMatrix / NewIdentityMatrix. However, I left ZeroMatrix/IdentityMatrix as operations, even though I was tempted to turn them into a single global function each; the reason is that the cvec package installs methods for the variants getting an example matrix, as an optimization: each of its matrices stores a "CVecClass" object, and it can then reuse these. Anyway, whatever the reasons, that means these need to stay operations for now, and if want to change it, we need to coordinate it with the cvec package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: discussion discussions, questions, requests for comments, and so on
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant