search

fatiando / rockhound

NOTICE: This library is no longer being developed. Use Ensaio instead (https://www.fatiando.org/ensaio). -- Download geophysical models/datasets and load them in Python
BSD 3-Clause "New" or "Revised" License
34 stars 15 forks source link

More 1D Earth Models from IRIS #30

Closed leouieda closed 2 years ago

leouieda commented 5 years ago

Description of the desired feature

There are a bunch of Earth Models in the IRIS website besides PREM (see #10). We could have functions to download and load many of those, which would be interesting to plot and see their differences. See http://ds.iris.edu/ds/products/emc-referencemodels/

These functions wouldn't be too complicated and load the models into pandas.DataFrame, much like the fetch_prem function.

ChetGoerzen commented 5 years ago

I am interested in helping with this. Would you like a function for all of the EMC-hosted reference Earth models? I plan on following a similar format to the fetch_prem function.

leouieda commented 5 years ago

Hi @ChetGoerzen, thanks for volunteering! I wonder if having a single function would make the documentation a bit cluttered. Ideally, the docstring should cite the reference for every model and include a description of each. It might make more sense to have separate functions for each one. The amount of code required is minimal and a majority of the work will be the docstrings (which we can't reuse between models anyway). What do you think?

@santisoler any opinions on this?

ChetGoerzen commented 5 years ago

Hi @leouieda, thanks for being so open to contributors. I think that having separate functions for each model sounds like a good idea. Would it be alright to include all these functions in one script? Also, would you like me to include all the earth models from [http://ds.iris.edu/ds/products/emc-referencemodels/]()?

leouieda commented 5 years ago

Would it be alright to include all these functions in one script?

That would be fine. You could even move the PREM code into it. We generally import all functions in rockhound/__init__.py so the internal organization of modules doesn't matter to the user.

Also, would you like me to include all the earth models

Sure, that would be great. You can start with one to get the process down and add more later. What do you think?

ChetGoerzen commented 5 years ago

Hi @leouieda, I am close to completing the work for this issue. I think it may be easiest for me to submit a pull request once I have completed everything. When writing the tests I have experienced some of the tests failing due to what appears to be floating point error. Replacing the standard assert statement by using the numpy.testing.assert_almost_equal with precision up to six decimal places seems to have solved this problem. Is it alright for me to write my tests using this function and this precision?

santisoler commented 5 years ago

Hi @leouieda and @ChetGoerzen. Sorry for the delay. I agree with adding more reference models. A few of them are widely used among seismologists, like the iasp91 or ak135-f. So would be nice to have them in Rockhound.

@ChetGoerzen would you mind opening that PR? It doesn't matter if it's not finished, you could add WIP on the PR title (WIP: Work In Progress) so the WIP bot doesn't allow us to merge. It's easier to review code and discuss it on the PR interface.

When comparing numerical values we use numpy.testing.assert_allclose(...) instead of assert ... == ..., you are right about that! See the test_seafloor_age() function for example.

Thanks for taking the time to tackle this down!