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.

Sign up for GitHub

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

Jump to bottom

Update point source use Choclo #422

Merged
merged 22 commits into from
Jan 13, 2024
Merged

Update point source use Choclo #422

merged 22 commits into from
Jan 13, 2024

Conversation

LL-Geo
Copy link
Member

@LL-Geo LL-Geo commented Jul 7, 2023

  1. Update point source use Choclo
  2. Rename the kernel from kernel_g_z_spherical to kernel_gravity_u_spherical
  3. Update tesseroid associate with kernel name change
  4. Update Test add Choclo

Relevant issues/PRs:

@LL-Geo
Update point source use Choclo
9bc2e04 
1. Update point source use Choclo
2. Rename the kernel from kernel_g_z_spherical to kernel_gravity_u_spherical
3. Update tesseroid associate with kernel name change
4. Update Test add Choclo
@LL-Geo
Revert "Update point source use Choclo"
8cea0f9 
This reverts commit 9bc2e04.
@LL-Geo
Update point source use Choclo
74f5671 
1. Update point source use Choclo
2. Rename the kernel from kernel_g_z_spherical to kernel_gravity_u_spherical
3. Update tesseroid associate with kernel name change
4. Update Test add Choclo
@LL-Geo
Update test
f6badba 
1. change northing and easting to n and e
2. Same for FFT test
@LL-Geo
Copy link
Member Author

LL-Geo commented Jul 7, 2023

Pushed into the wrong branch at the beginning 😅..

Should be able to go... I think I will work on the Choclo for the spherical kernel when I find time :)

Rename g_northing to g_n and g_easting to g_e!

@LL-Geo LL-Geo requested a review from santisoler July 7, 2023 16:21
santisoler
santisoler requested changes Jul 19, 2023
View reviewed changes
Copy link
Member

@santisoler santisoler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LL-Geo for this PR and sorry for the delayed review. I think it looks great!

I left some suggestions. Most of them are related to some changes made in one of the latest PRs being reverted (mostly docstrings), and I left a few to improve some styling.

Let me know what do you think!

harmonica/_forward/point.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/tests/test_point_gravity.py Outdated Show resolved Hide resolved
harmonica/_forward/point.py Outdated Show resolved Hide resolved
LL-Geo and others added 8 commits September 7, 2023 13:19
@LL-Geo @santisoler
Update harmonica/_forward/point.py
82acf52 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo @santisoler
Update harmonica/_forward/tesseroid.py
83361c8 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo @santisoler
Update harmonica/_forward/tesseroid.py
d0c091f 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo @santisoler
Update harmonica/_forward/tesseroid.py
f2a291d 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo @santisoler
Update harmonica/_forward/tesseroid.py
1460341 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo @santisoler
Update harmonica/_forward/tesseroid.py
4c8e995 
Co-authored-by: Santiago Soler <[email protected]>
@LL-Geo
Update code after review
6631471 
1. rename kernel_gravity_u_spherical to gravity_u_spherical
2. clean test
3. two stage units for tesseroid
@LL-Geo
Merge branch 'main' into point_source
2945ef4
@LL-Geo
Copy link
Member Author

LL-Geo commented Sep 7, 2023 •
edited
Loading

Thanks @santisoler for the review 😄 It looks like it is in good shape.
There is an error pup up from the test related to pyVista:

FAILED ..\tests\test_visualizations.py::test_prism_to_pyvista[numpy] - AttributeError: 'UnstructuredGrid' object has no attribute 'cell_bounds'
FAILED ..\tests\test_visualizations.py::test_prism_to_pyvista[xarray] - AttributeError: 'UnstructuredGrid' object has no attribute 'cell_bounds'
========== 2 failed, 466 passed, 126 skipped, 80 warnings in 57.42s ===========
mingw32-make: *** [makefile:32: test_coverage] Error 1
Error: Process completed with exit code 2.

I'll have a look later

@santisoler santisoler mentioned this pull request Sep 7, 2023
Merged
@santisoler
Copy link
Member

Thanks for reporting it @LL-Geo! I have fixed it in #433, feel free to update this branch whenever you want.

@santisoler santisoler added this to the v0.7.0 milestone Sep 8, 2023
@LL-Geo
Copy link
Member Author

LL-Geo commented Dec 6, 2023

Greetings from another continent... I guess this one is ready to merge... :)

@santisoler santisoler removed their request for review December 6, 2023 17:56
@santisoler santisoler self-requested a review December 6, 2023 17:56
santisoler
santisoler requested changes Jan 10, 2024
View reviewed changes
Copy link
Member

@santisoler santisoler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great! I think we should merge it.

I just made a small suggestion, see below.

harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
harmonica/_forward/tesseroid.py Outdated Show resolved Hide resolved
@LL-Geo
Copy link
Member Author

LL-Geo commented Jan 13, 2024 •
edited
Loading

Thanks @santisoler :)

I'll merger that, haha

@LL-Geo LL-Geo merged commit bbaf5a1 into main Jan 13, 2024
20 checks passed
@LL-Geo LL-Geo deleted the point_source branch January 13, 2024 07:01
@santisoler
Copy link
Member

Is this our first PR merged from Antarctica?? 🐧 ❄️

@LL-Geo
Copy link
Member Author

LL-Geo commented Jan 20, 2024

Is this our first PR merged from Antarctica?? 🐧 ❄️

Haha, Yeppp~ 🐧

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@santisoler santisoler santisoler approved these changes

Assignees

@LL-Geo LL-Geo

Labels
None yet
Projects
Harmonica's forward modelling
Status: Done
Milestone
v0.7.0
Development

Successfully merging this pull request may close these issues.
2 participants
@LL-Geo @santisoler