-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added CGLS using DistributedArray #29
Conversation
Minor: change in doc
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 code looks very good. I just found something in the tests that I didn't like (the use of a very loose assert assert_array_almost_equal(.. decimal=4)
and tried to switch to assert_allclose(..., rtol=1e-14)
- I remember Yuxi asked something similar for the tests of DistributedArray.
I tried to make the change but the tests fail (some pass if I lift it to 1e-8)... this tells me something isn't the same as the pylops implementation. As long as we understand why this is the case, it is not a problem (perhaps MPI does some rounding when passing arrays?), but I would like you to investigate this a bit before merging and being happy with having some inconsistency with the serial solver
So @mrava87 I was able to figure out as to why it was failing for |
Ok, interesting. Good finding :) then I think this is good to go |
conj()
andcopy()
toDistributedArray
as they were used quite often.CGLS
class similar to pylops but now usingDistributedArray
.cgls
method which will be used by users, as we do in pylops(again withDistributedArray
).cgls
andCGLS
to docs.cgls
to__init__.py
(same as pylops).