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

Better default for depth in equivalent sources #424

Open
leouieda opened this issue Jul 27, 2023 · 4 comments · May be fixed by #491
Open

Better default for depth in equivalent sources #424

leouieda opened this issue Jul 27, 2023 · 4 comments · May be fixed by #491
Assignees
Labels
enhancement Idea or request for a new feature
Milestone

Comments

@leouieda
Copy link
Member

Description of the desired feature:

From chats with @indiauppal and @birocoles, I learned that Dampney (1969; the original equivalent source paper) advises using depths between 2.5 and 6 times the source spacing (assuming sources in a regular grid). For sources beneath data, like we use here, a sensible default is 4.5 times the median distance between sources. Me and @indiauppal tested this default and it works well most of the time. Tweaking is always good but this would provide a more sensible default than the fixed value we currently use.

This would break backwards compatibility. I'm not too worried since our docs say that you should always set the depth by hand. So changing the default will likely make the interpolation better for the few people who were using the classes without following our advice. But I can do the warning dance if others think it's necessary (but it will take longer to implement).

Are you willing to help implement and maintain this feature?

Yes. But very happy to let someone else do it (😉 @indiauppal)

@leouieda leouieda added the enhancement Idea or request for a new feature label Jul 27, 2023
@santisoler santisoler added this to the v0.7.0 milestone Jan 26, 2024
@leouieda
Copy link
Member Author

@santisoler would you like me to first make a PR with a warning that the default will change that can go into v0.7 and then we make the change for v0.8?

@santisoler santisoler self-assigned this Apr 16, 2024
@santisoler
Copy link
Member

santisoler commented Apr 16, 2024

Just to clarify, the depth should be set to 4.5 times the median distance between first neighbor sources, right?

If that's true, then we can use:

depth = np.median(verde.median_distance(coordinates, k_nearest=1))

Is this the idea?

@indiauppal
Copy link
Member

Should we take the mean of the median distances?
depth = np.mean(verde.median_distance(coordinates, k_nearest=1)) * 4.5

@leouieda
Copy link
Member Author

I guess the mean would be better? Then it wouldn't favor close points. Perhaps it would be good to have a verde.mean_distance function as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Idea or request for a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants