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

Change default value of depth in equivalent sources #491

Merged
merged 17 commits into from
Jun 12, 2024
Merged

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented Apr 16, 2024

Change the default value of the depth argument in EquivalentSources to "default". If this value is passed, then the depth of the sources is set as 4.5 times the mean distance between first neighbouring sources. If a numerical value is passed, this is the one that will be used. Introduce a new depth_ attribute where the estimated/passed numeric value is stored. If points is passed, the depth_ attribute is set to None. Add a test checking that the depth value is properly set. Merge the tests functions that checked the accuracy of equivalent sources using different dtypes.

Relevant issues/PRs:

Fixes #424

Change the default value of the `depth` argument in `EquivalentSources`
to `"default"`. If this value is passed, then the depth of the sources
is set as 4.5 times the median distance between first neighbouring
sources. If a numerical value is passed, this is the one that will be
used. Introduce a new `depth_` attribute where the estimated/passed
numeric value is stored. If `points` is passed, the `depth_` attribute
is set to None. Add a test checking that the depth value is properly
set.
Merge the tests functions that checked the accuracy of equivalent
sources using different dtypes.
@santisoler
Copy link
Member Author

I should update some of the tutorials, so they make use of the default depth strategy. We can also include some information on why setting the depth to a numerical value and its consequences.

Add admonition explaining the default behaviour of the ``depth``
argument.
@santisoler santisoler marked this pull request as ready for review May 3, 2024 22:55
@santisoler santisoler requested a review from leouieda May 3, 2024 22:55
@santisoler
Copy link
Member Author

@indiauppal would you like to check this PR out?

Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Fixed a small typo. But otherwise, this is great!

harmonica/_equivalent_sources/cartesian.py Outdated Show resolved Hide resolved
@santisoler santisoler merged commit 50b87f9 into main Jun 12, 2024
19 checks passed
@santisoler santisoler deleted the eqs-default-depth branch June 12, 2024 18:56
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.

Better default for depth in equivalent sources
2 participants