search

slimgroup / JUDI.jl

Julia Devito inversion.
https://slimgroup.github.io/JUDI.jl
MIT License
96 stars 30 forks source link

fix mul! for JOLI operator and judiVector #114

Closed ziyiyin97 closed 2 years ago

ziyiyin97 commented 2 years ago

This PR makes

lsqr!(x, J*Mr, dobs)

work. Especially this works when Mr takes complex input (e.g. debiasing for curvelet)

codecov[bot] commented 2 years ago

Codecov Report

Merging #114 (867f9d4) into master (850f4b5) will decrease coverage by 0.03%. The diff coverage is 50.00%.

:exclamation: Current head 867f9d4 differs from pull request most recent head 7f2b6c3. Consider uploading reports for the commit 7f2b6c3 to get more accurate results

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
- Coverage   82.42%   82.39%   -0.04%     
==========================================
  Files          24       24              
  Lines        1861     1863       +2     
==========================================
+ Hits         1534     1535       +1     
- Misses        327      328       +1
Impacted Files Coverage Δ
src/TimeModeling/LinearOperators/operators.jl 81.57% <33.33%> (-1.46%) :arrow_down:
src/TimeModeling/Types/abstract.jl 77.96% <66.66%> (ø)
src/TimeModeling/LinearOperators/basics.jl 77.27% <0.00%> (+2.27%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 850f4b5...7f2b6c3. Read the comment docs.

ziyiyin97 commented 2 years ago

Without (msv::judiMultiSourceVector{mT})(x::AbstractVector{T}) where {mT<:Number, T<:Number} = x then https://github.com/slimgroup/JUDI.jl/blob/e22998d41b3e368ce015ef5ed0f76bddfbade455/test/test_linear_algebra.jl#L101 is not passing. I do think we need to add this because again x and msv might live in different space.

mloubout commented 2 years ago

. I do think we need to add this because again x and msv might live in different space.

Then need to be added but properly:

ziyiyin97 commented 2 years ago

any other comment on this one?

ziyiyin97 commented 2 years ago

Hmm the memory blows up in the CI test https://github.com/slimgroup/JUDI.jl/runs/7285219825?check_suite_focus=true any suggestion?

ziyiyin97 commented 2 years ago

Correct me if wrong but this parallelization seems to be the culprit https://github.com/slimgroup/JUDI.jl/blob/01eedad195f2a52d608ad7ee183f0cf492d20696/src/TimeModeling/Modeling/distributed.jl#L54 because in principal it shouldn't insist to run the born/migration with all sources if not enough memory

mloubout commented 2 years ago

Correct me if wrong but this parallelization seems to be the culprit

I have no idea why this would be related to it this just reduces results pair by pairs.

mloubout commented 2 years ago

Also once again, please learn to rebase your branches I cannot do it for you every time.