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

Missing attributes on ICGEM datasets #125

Closed
santisoler opened this issue Nov 8, 2019 · 8 comments · Fixed by #184
Closed

Missing attributes on ICGEM datasets #125

santisoler opened this issue Nov 8, 2019 · 8 comments · Fixed by #184
Labels
bug Report a problem that needs to be fixed
Milestone

Comments

@santisoler
Copy link
Member

santisoler commented Nov 8, 2019

Description of the problem

We currently have three sample datasets that have been downloaded from the ICGEM Calculation Service:

  • Earth Topography
  • Earth Gravity
  • Earth Geoid

Each dataset can be fetched through its own function. Every one of these functions claims to have more details regarding the generation of the grid under the attrs attribute of the Dataset.
The problem is neither of the returned Datasets have non empty attrs.

Would be nice to correct this and include the attributes related to the grid generation.
I think these grids where generated by @leouieda, so maybe he has more idea of what information should be included on the attrs.

Just by naming some possible attributes that appear on the ICGEM Calculation Service:

  • Reference ellipsoid
  • Height above the ellipsoid
  • Grid step
  • Model used (for gravity and geoid)
  • Tide system
  • Zero degree term (yes/no)
  • Gaussian filter and filter length
  • Low-pass filtering
    • Start gentle cut
    • Maximum degree
@santisoler santisoler added bug Report a problem that needs to be fixed help wanted labels Nov 8, 2019
@leouieda
Copy link
Member

That's strange. I was sure they included those. All that info is included in the ICGEM header and we load it into attrs in load_icgem_gdf. So the process should be as simple as downloading new grids from ICGEM, reading them in with harmonica.load_icgem_gdf, saving them to netCDF, compressing the netCDF with lzma.

@santisoler
Copy link
Member Author

Ok. I think we should create a single PR for solving this issue and #124, taking into account that we should download the ICGEM grids again.

@nshea3
Copy link
Member

nshea3 commented Dec 10, 2019

Hi all,

I'll give these two issues a try (#124 and #125) before I forget how the data fetching module works.
I'll be in touch if I have any questions!

Nick

@santisoler
Copy link
Member Author

Hi @nshea3! Thanks for helping out with these two issues.
Feel free to open a PR whenever you're ready, or anytime you want to share your advances or get any help.
Good luck with this!

@santisoler santisoler added this to the v0.2.0 milestone Jan 27, 2020
@santisoler
Copy link
Member Author

@nshea3 have you made any advances on this?

I'm thinking we might want to solve this (along with #124) before completely moving the datasets to Rockhound, which won't live inside its repository, but on a Zenodo archive (cc @mtb-za).

Please, @nshea3 don't feel pushed to work on it. Just do it if you have the time. Otherwise, please let me know.

@leouieda
Copy link
Member

We should also set the long_name and units attributes. xarray picks these up and plots them automatically.

@nshea3
Copy link
Member

nshea3 commented Jul 21, 2020

Hi all! This one slipped my mind during the spring. I have not started on it yet but I have plenty of time to work on it this week. I'll let you know how I progress.

@nshea3
Copy link
Member

nshea3 commented Jul 28, 2020

I believe I made some progress on this one:

Many of those attributes are included in the netcdf files, however they are dropped in the fetch_* functions in sample_data.py with the float64 cast: data = xr.open_dataset(fname, engine="scipy").astype("float64"). When astype("float64") is removed the attributes show up in the xarray object. With this change made, all tests pass and the examples still run.

I'm still putting together the pull request but I wanted to send along an update!

Thanks,

Nick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report a problem that needs to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants