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 GEOS-IT option for remap restarts #96

Merged
merged 15 commits into from
Sep 30, 2024
Merged

Conversation

biljanaorescanin
Copy link
Contributor

@biljanaorescanin biljanaorescanin commented Sep 9, 2024

Added remapping from GEOS-IT restarts.
GEOS-IT has only two dates available for users to be remapped, day 14th and day 28th, and they are in .tar format.

This PR will address these two issues:
#62
#72

PR was tested for a run using the on-screen option for GEOS-IT vs. copying and un-tar files manually and using the tool to remap my restarts. The difference is zero diff.

@biljanaorescanin biljanaorescanin added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Sep 9, 2024
@mathomp4
Copy link
Member

mathomp4 commented Sep 9, 2024

Well that was a lot more than I was expecting for this! Great job!

@biljanaorescanin
Copy link
Contributor Author

PR was tested for diff input source restarts; M2, GEOS-IT and user owned restarts remapping.
PR was also tested with automated tests on SLES12 and that is zero diff as well.

@weiyuan-jiang I've added agcm rst remap since I didn't want to confuse users if we didn't do that with GEOS-IT, this way it is same principal as M2 handling.

@biljanaorescanin biljanaorescanin marked this pull request as ready for review September 17, 2024 14:49
@biljanaorescanin biljanaorescanin requested review from a team as code owners September 17, 2024 14:49
@weiyuan-jiang
Copy link
Contributor

PR was tested for diff input source restarts; M2, GEOS-IT and user owned restarts remapping. PR was also tested with automated tests on SLES12 and that is zero diff as well.

@weiyuan-jiang I've added agcm rst remap since I didn't want to confuse users if we didn't do that with GEOS-IT, this way it is same principal as M2 handling.

The problem is if the users answer No to remap agcm_import _rst, it will be copied and remapped anyway.

@biljanaorescanin
Copy link
Contributor Author

PR was tested for diff input source restarts; M2, GEOS-IT and user owned restarts remapping. PR was also tested with automated tests on SLES12 and that is zero diff as well.
@weiyuan-jiang I've added agcm rst remap since I didn't want to confuse users if we didn't do that with GEOS-IT, this way it is same principal as M2 handling.

The problem is if the users answer No to remap agcm_import _rst, it will be copied and remapped anyway.

Good catch, I've fixed it.

@biljanaorescanin
Copy link
Contributor Author

I've found trying out different dates loop was not good. I was testing just with resent dates. Now it works from 2007 01.

@weiyuan-jiang weiyuan-jiang marked this pull request as draft September 17, 2024 17:13
@biljanaorescanin
Copy link
Contributor Author

@weiyuan-jiang testing case c24Toc12 will fail since we now set false for default for agcm_import_rst remapping. It was just that base case did have that remapped and current doesn't so file count comparison fails.

But we could just remove that restarts from base? Or change default ?

All other testing was ok. I didn't find any more issues.

@gmao-rreichle
Copy link
Contributor

cc @gmao-rreichle

@biljanaorescanin biljanaorescanin marked this pull request as ready for review September 25, 2024 16:02
@biljanaorescanin
Copy link
Contributor Author

biljanaorescanin commented Sep 25, 2024

I've repeated testing all look fine for my cases and automated tests. I suppose it is ready for you @sdrabenh

Also everyone is welcome to try cases they use most often I just test for some of the options I can't always test it all.

@sdrabenh sdrabenh merged commit 27d74aa into main Sep 30, 2024
13 checks passed
@sdrabenh sdrabenh deleted the feature/borescan_add_geosit branch September 30, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants