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

Add option for deep atmosphere #1606

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Add option for deep atmosphere #1606

merged 1 commit into from
Feb 9, 2024

Conversation

Sbozzolo
Copy link
Member

@Sbozzolo Sbozzolo commented Feb 5, 2024

This is a rebased and cleaned (from a git perspective) version of #1591 (authorship is preserved).

This PR adds supports for deep atmospheres and changes a little bit the hypsography interface.

I am not particularly satisfied with the current hypsography interface, in particular in relation with the interpolation methods.

Among the problems are:

  • Converstions between scalars and ZPoints. The interpolation interface works with scalars, the hypsography with ZPoints, so there are conversions that need to be done between the two
  • Restarting from HDF5 currently goes through branches that are hardcoded for the hypsographies we support, but there is no reason why this should be the case.
  • At the moment, both the "reference to physical z" and "physical to reference z" transformations are required, but some coordiante transformations do not have closed forms for both (e.g., our SLEVE), so that some features in the interpolation interface cannot be easily implemented.

In the interest of merging support for deep atmosphere, we can go ahead with this PR and tackle the issues with hypsography/interpolation in the future.

Closes #1524

@Sbozzolo Sbozzolo force-pushed the gb/new-deep-sphere branch 3 times, most recently from b378096 to 528af66 Compare February 6, 2024 00:23
@akshaysridhar akshaysridhar marked this pull request as ready for review February 7, 2024 20:00
Copy link
Member

@akshaysridhar akshaysridhar left a comment

Choose a reason for hiding this comment

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

Thanks @Sbozzolo. This is ready to rebase and merge with the included changes against commit c81d857.

@akshaysridhar akshaysridhar mentioned this pull request Feb 8, 2024
6 tasks
@akshaysridhar akshaysridhar linked an issue Feb 8, 2024 that may be closed by this pull request
Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

I left a few more comments, but overall looks good.

@akshaysridhar
Copy link
Member

I left a few more comments, but overall looks good.

Thanks @charleskawczynski ! My opinion is that the questions about Point support be addressed in future PRs related to diagnostics / interpolations (type conversion support etc.) . I have merged your suggestions w.r.t dss bufers and alloc reduction in the smoothing function.

@Sbozzolo
Copy link
Member Author

Sbozzolo commented Feb 9, 2024

I rebased, squashed, and addressed the comment on map. I don't know the answer for ZPoint. Good to go on my end.

Co-authored-by: Akshay Sridhar <asridhar@caltech.edu>
Co-authored-by: Gabriele Bozzola <gbozzola@caltech.edu>
@Sbozzolo Sbozzolo enabled auto-merge February 9, 2024 17:24
@Sbozzolo Sbozzolo merged commit 028603d into main Feb 9, 2024
15 checks passed
@Sbozzolo Sbozzolo deleted the gb/new-deep-sphere branch February 9, 2024 20:01
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.

Hypsography is not GPU-compatible
4 participants