Skip to content

What to do when @earth_relief_xxy is given? #53

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

Closed
PaulWessel opened this issue May 30, 2020 · 15 comments · Fixed by #77
Closed

What to do when @earth_relief_xxy is given? #53

PaulWessel opened this issue May 30, 2020 · 15 comments · Fixed by #77

Comments

@PaulWessel
Copy link
Member

Here is my proposal as to what we do when remote files are specified without a choice for registration.

  • GMT <= 6.0.0: Links on the server pointing to the gridline-registered versions in the deeper directories. That way we stay backwards compatible with 6.0.0. Since 6.0.0 does not read gmt_data_server.txt it cannot access the other files on the server.
  • GMT 6.1: Principle is to avoid duplicating data. Thus, I suggest that we do the same thing we do if there is no extension: We append it. Hence, a given filename of @earth_relief_02m should first have "_p" appended to it (or g if there is no p), and then we append ".grd" to make it a file name. This way there is never a file called earth_relief_02m.grd written to the users directory, only the one in the earth/earth_relief directory. If earth_relief_xxy is actually a directory with tiles, we download the needed tiles, blend and return that grid. Therefore, we should not add links inside the earth/earth_relief directory to deal with the missing p or g as that only would lead to duplication of files or require a more complicated implementation in GMT. We do not want (or can) to set symbolic links in the user's directory. I think a consequence of this for 6.1 is that we ignore any old grids. The reason is that they are already outdated by newer data and if they had the right name and placement they would be re-downloaded anyway.
  • Error: Someone giving @earth_relief_xxy_p|g when there is no such file or directory.
@seisman
Copy link
Member

seisman commented May 30, 2020

It's a good and clean solution to me.

@seisman
Copy link
Member

seisman commented Jun 19, 2020

@PaulWessel @weiji14 Related to GenericMappingTools/pygmt#451:

  • GMT <= 6.0.0: Links on the server pointing to the gridline-registered versions in the deeper directories. That way we stay backwards compatible with 6.0.0. Since 6.0.0 does not read gmt_data_server.txt it cannot access the other files on the server.

Currently, the earth relief data for 6.0.0 are pointing to the pixel-registered grids.

@PaulWessel
Copy link
Member Author

Not sure I understand. We have links in place for GMT 6 so that they can still get grids. Yes, they now point to pixel grids. What is the problem with this?

@seisman
Copy link
Member

seisman commented Jun 19, 2020

It's a breaking change for GMT 6.0.0, although most users may never notice the changes of the grid registrations.

Currently, it's a big problem for PyGMT, since now PyGMT can only pass gridline grids to GMT API.

@PaulWessel
Copy link
Member Author

I am sorry I am slow to grasp this. Is there a limitation in pyGMT that means it can only handle gridline-registered grids? I.e., when pyGMT now tries to read @earth_relief_30m and it is a pixel-grid it somehow fails due to a code limitation or a hardwired setting? I am just trying to understand what the problem is and I dont, yet. Bedtime for me, so more tomorrow.

@weiji14
Copy link
Member

weiji14 commented Jun 19, 2020

It's ok, I'll try to work it out on the Py side first. I think I have a better idea what's happening now after reading this thread.

@weiji14
Copy link
Member

weiji14 commented Jun 26, 2020

  • GMT <= 6.0.0: Links on the server pointing to the gridline-registered versions in the deeper directories.

Does this mean there is still a way to get the gridline registered earth_relief grids that were the default in GMT 6.0.0? What is meant by deeper directories here? It seems like GMT 6.0.0 can only download the pixel registered grids now, and gmt which @earth_relief_01d_g doesn't seem to work.

@PaulWessel
Copy link
Member Author

6.1.x can read @earth_relief_01d_g but 6.0.0 has no idea about where those are. It actually reads @earth_relief_01d_p.grd via a symbolic link called earth_relief_01d.

@weiji14
Copy link
Member

weiji14 commented Jun 26, 2020

Ok, so this sounds like backward compatibility is broken then? Considering that GMT 6.0 cannot handle pixel registered grids via the C API (GenericMappingTools/gmt#3502, GenericMappingTools/gmt#3503), would it be better to change the symlink to point to the *_g.grd files instead, or was there another reason to point them to the *_p.grd files?

@PaulWessel
Copy link
Member Author

Yes, we went back and forth on this for a while (where should the pointer point). Given the problem I have no objection to changing the pointer to provide gridline grids for 6.0.0. Do you see an issue with this, @seisman?

@seisman
Copy link
Member

seisman commented Jun 26, 2020

Changing them to gridline grids will make definitely @weiji14 and me happy.

@seisman
Copy link
Member

seisman commented Jun 26, 2020

@weiji14 I think that depends on if we want to have a PyGMT v0.1.2 release.

Besides the grid registration change, the precision of the elevation data also changed in recent server updates. The old earth relief data may have a maximum elevation of 6345.3489, while the new data have a maximum of 6345.5. If we want to have a v0.1.2 release, we still need to update all the baselines images to pass all the tests. After GMT 6.1.0 is released, we need to release v0.2.0 ASAP, again we need to update the baseline images due to the registration change.

On the other hand, if we just want to release v0.2.0, which requires GMT>=6.1.0, we only need to update the baseline images once.

@weiji14
Copy link
Member

weiji14 commented Jun 26, 2020

Yes, in any case, we'll need to update the baseline png images at least once. But we probably don't have to do it twice if we're careful. The tests can still point to gridline images in GMT 6.1, so we could keep the old tests as gridline only, and new ones we add can use pixel registration. We can discuss this further in the meeting later with Liam if there's time.

@weiji14
Copy link
Member

weiji14 commented Jun 26, 2020

Thank you very much Paul! DongDong mentioned at GenericMappingTools/pygmt#451 (comment) that you've symlinked the earth_relief_*.grd images back to gridline registered for GMT6.0. I see the PRs at #64 and #65. Will go and enjoy my weekend now 😄

@PaulWessel
Copy link
Member Author

You bet. Let me know if anything fishy is found.

seisman added a commit that referenced this issue Nov 1, 2020
This PR documents the important discussions in #37, #48, #53.

Closes #37.
Closes #48.
Closes #53.
Closes #63.
PaulWessel pushed a commit that referenced this issue Nov 1, 2020
This PR documents the important discussions in #37, #48, #53.

Closes #37.
Closes #48.
Closes #53.
Closes #63.
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 a pull request may close this issue.

3 participants