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.

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

Simplify tests for upward derivative #328

Merged
merged 3 commits into from
Jul 6, 2022

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented Jun 27, 2022

Description

Ditch the precomputed sample grid file and add new fixtures for creating
a pair of sample sources, a sample grid and some gravity fields as
DataArrays. Remove netcdf4 from testing dependencies.

Relevant issues/PRs

Related to #299

Ditch the precomputed sample grid file and add new features for creating
a pair of sample sources, a sample grid and some gravity fields as
DataArrays.
@santisoler santisoler requested a review from LL-Geo June 27, 2022 16:02
@santisoler
Copy link
Member Author

@LL-Geo how does it looks now? Does it makes a good boiler plate for the test you're working on in #299 ?

@LL-Geo
Copy link
Member

LL-Geo commented Jun 28, 2022

@LL-Geo how does it looks now? Does it makes a good boiler plate for the test you're working on in #299 ?

Hey @santisoler , that looks very good for me! I'm thinking to add a little complexity to the test. It might be good to put a pair of sources with positive and negative density contrast and depth.

points = [
[-50e3, 50e3],
[-50e3, 50e3],
[-20e3, -20e3],
Copy link
Member

Choose a reason for hiding this comment

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

might be good to have one shallow source and one deep source?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! Well thought!

[-20e3, -20e3],
]

masses = [3.3e8, 2.9e8]
Copy link
Member

Choose a reason for hiding this comment

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

A positive and negative mass might be good as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed! You're right that the model should be a little bit more complex.

@santisoler
Copy link
Member Author

Thanks @LL-Geo for the review! Making the sample model more complex was a clever idea. I'm merging this!

@santisoler santisoler merged commit 9d8531d into main Jul 6, 2022
@santisoler santisoler deleted the simplify-transformations-tests branch July 6, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants