-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix 80 #223
base: master
Are you sure you want to change the base?
Fix 80 #223
Conversation
Still trying to get original mom6_grid_spec removed
for more information, see https://pre-commit.ci
Was not intended to be included to begin with.
…t pole_kind option.
I noticed that the pole_kind option was introduced in ESMF 8.0.1, so added a bit of code to handle earlier versions. From my perspective, the remaining open question is testing. The test data included in this PR weighs about 57M. I don't think it's a good idea to include such large test files in the main branch. Can we generate synthetic grids on the fly and test on those instead ? |
Hi @huard - Yes, the size of the testing files worried me also. Does xesmf have the capability to generate a tripole grid on the fly? I noticed that there was a "mom6_like" grid being generated in the tests, but I didn't look to see if it was the same as the tripole. If it is then we should be able to drop the test files. I notice that all of the build tests are failing - is this something I need to address? Looking briefly at the details it wasn't obvious to me what the issue was. |
I'll let Raphael answer the first question. |
Build failures are due to a bug with |
Alternatively, this workaroudn should fix things if you'd like something working sooner: nextstrain/cli@4a76497 |
Thanks ! |
Any word on creating a tripole grid on the fly to test? |
I think a good way to test would be to generate the bipole northern cap using a code derived from https://github.com/NOAA-GFDL/ocean_model_grid_generator/blob/2563e8e69e170a761fb409aab7addd6faf83cecf/ocean_grid_generator.py#L115 I don't have time to look at this right this moment but I can look into it next week |
I just added a function to build a simple tripolar grid from scratch. That will allow us not to add large files Let me know if you have any questions or need help with the new PR. Thanks again for your contribution and I am really looking forward to adding this option!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this branch contains large files and needs to be replaced by a new one without files and using util functions to create grids
I think git squash rewrites the history, so the large files would not be committed to the main branch. To be confirmed. |
I don't master git (pun intended) enough to confirm/infirm. Good news is that there are not that many code changes and they could be copied to a new branch in 30 seconds with meld/vimdiff. The most time consuming part would be to rewrite the tests. @benjamin-cash let us know if you need help with this |
Hi @raphaeldussin @huard - I've been on vacation the past week so am getting caught up today. Did anyone determine whether I can git rm the large files and not have them committed or if I need to create a new branch? Determining that answer is also beyond my git skillz. |
@benjamin-cash hope you had a great vacation and welcome back. Personally I would just create a new branch and copy the code changes into it. |
The docs mention squash is a "rebase" operation, so I'm pretty confident squash and merge will not store the history with binary files. |
@benjamin-cash I did a rebase locally and pushed to a new PR. See #236. Please make sure I did not miss something in the operation. |
Description
Modified specification of grids to allow for use of "pole_kind" specification available in ESMF to properly account for tripolar grids (e.g. MOM6).
Type of change
New feature (non-breaking change which adds functionality)
How has this been tested?
pytests on Frontera HPC used to confirm non-breaking status, comparison to output of previous method to confirm expected behavior of new method is being seen.
Checklist
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
New and existing tests pass with my changes
Any dependent changes have been merged and published