slimgroup / JUDI.jl

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

Fix non-zero origin in `limit_model_to_receiver_area` func #113

Closed kerim371 closed 2 years ago

kerim371 commented 2 years ago

Before that limit_m=true option sometimes used to lead to exception. The cause was the typo within limit_model_to_receiver_area function. Now the typo is fixed and model origin is taken into account

codecov[bot] commented 2 years ago

Codecov Report

Merging #113 (f60a864) into master (76e7ba3) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #113   +/-   ##
=======================================
  Coverage   81.55%   81.55%           
=======================================
  Files          23       23           
  Lines        1816     1816           
=======================================
  Hits         1481     1481           
  Misses        335      335           
Impacted Files Coverage Δ
src/TimeModeling/Utils/auxiliaryFunctions.jl 77.67% <100.00%> (ø)

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 76e7ba3...f60a864. Read the comment docs.

kerim371 commented 2 years ago

@ziyiyin97 hi,

I have no experience of working with julia tests so please help me to organize it correctly (tell me where to put the code, probably I need to create another file?). Also I have not tried to run it. If you know please show the command that I can use to run my test

kerim371 commented 2 years ago

@ziyiyin97 I tried to follow your recommendations and it seems better now but the functions example_rec_geometry and example_crs_geometry are for 2D. I decided to not use them.

But it general it should be fine now

ziyiyin97 commented 2 years ago

Hey @kerim371 I am very sorry but @mloubout just reminded me that there is already test in here https://github.com/slimgroup/JUDI.jl/blob/76e7ba33de3b81ae76c2767c49ca270a3fcf0b36/test/test_basics.jl#L9 just need to change o to a non-zero one. Could you make the change? Apologies for wasting your time.

kerim371 commented 2 years ago

@ziyiyin97 ok, few notes:

change o = Tuple(0. for i=1:ndim) to say 10 leads to modifying: https://github.com/slimgroup/JUDI.jl/blob/76e7ba33de3b81ae76c2767c49ca270a3fcf0b36/test/test_basics.jl#L63

To check the restricted model coordinates I add: https://github.com/slimgroup/JUDI.jl/blob/b85fcfd4e02926e030fb14e3741f314fb8ca158a/test/test_basics.jl#L68-L74

Functions example_rec_geometry() and example_src_geometry() set y to zero: https://github.com/slimgroup/JUDI.jl/blob/76e7ba33de3b81ae76c2767c49ca270a3fcf0b36/test/utils.jl#L137-L151

Thus I decided to check only x coordinate

Just in case you encountered the same problem: I'm unable to run JUDI tests (my python is custom build but usually everything works):

GROUP=JUDI Julia --project -e 'using Pkg;Pkg.test(coverage=false)'
Fatal Python error: init_import_size: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
  File "/home/kerim/Documents/Colada/d/python-install/lib/python3.9/site.py", line 73, in <module>
    import os
  File "/home/kerim/Documents/Colada/d/python-install/lib/python3.9/os.py", line 29, in <module>
    from _collections_abc import _check_methods
  File "/home/kerim/Documents/Colada/d/python-install/lib/python3.9/_collections_abc.py", line 12, in <module>
    GenericAlias = type(list[int])
TypeError: 'type' object is not subscriptable

Did you encountered the same error?

ziyiyin97 commented 2 years ago

No that command runs fine for me. I suggest you check your python path, like https://stackoverflow.com/questions/65670259/every-python-related-executable-broken-fatal-python-error-init-import-size-f

kerim371 commented 2 years ago

@ziyiyin97 hi,

I can see that the BASICS test failed. DId I broke it or it was already broken?