search

slimgroup / JUDI.jl

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

dot test passes in assert only if using .data for viscoacoutic case (PR #95) #104

Closed nogueirapeterson closed 2 years ago

nogueirapeterson commented 2 years ago

Hi guys,

In our PR #95, we found an issue with respect dot function. We noticed that the dot test only passes in assert when we use .data in the input argument.

Attached is an MFE mfe.txt. Run with: julia mfe.jl --viscoacoustic

ziyiyin97 commented 2 years ago

Hi thanks again for your contribution. In JUDI the dot product/norm of judiVector is scaled by a time sampling rate https://github.com/slimgroup/JUDI.jl/blob/cada5b33192a289ee62221d061ddbba1e82c46c9/src/TimeModeling/Types/judiVector.jl#L361 so I would double check if this time-sampling is implemented correctly in your viscoacoustic implementation.

mloubout commented 2 years ago

As francis pointed out, object are dimensionalized. This is related to this comment on your PR:

https://github.com/slimgroup/JUDI.jl/pull/95/files#r845700881

The gradient is accumulated with a dimensionalized discrete intergral that needs the dt so you need to use the w like for the other cases and then you should not need the .data

nogueirapeterson commented 2 years ago

@ziyiyin97 and @mloubout thank you for the explanation. Now the test works without the .data.

mloubout commented 2 years ago

Great closing