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

support for catchcn ensemble run #584

Merged
merged 17 commits into from
Oct 18, 2022

Conversation

weiyuan-jiang
Copy link
Contributor

@weiyuan-jiang weiyuan-jiang commented Sep 22, 2022

Add CatchCN-specific export variables to EnsGridComp.
Required to support ensemble simulations with CatchCN in GEOSldas needed by @gmao-jkolassa.

Dependencies:

Prior to merging, change components.yaml accordingly.

After merging, change the following input files for the LDAS Nightly tests:

/discover/nobackup/mathomp4/LDAS_Restarts/NGHTLY_TST_TV4000/HISTORY_L4_SM.rc
/discover/nobackup/mathomp4/LDAS_Restarts/NGHTLY_TST_CS/assim/ldas179b8_C180/run/HISTORY.rc

Specifically, replace VEGDYN0000 with VEGDYN_e0000

@weiyuan-jiang
Copy link
Contributor Author

@gmao-jkolassa , if you want to test ensemble run, please use this fresh checkout of the branch .

@gmao-rreichle gmao-rreichle changed the title Feature/wjiang/catchcn ensemble new support for catchcn ensemble run Sep 22, 2022
@gmao-rreichle gmao-rreichle marked this pull request as draft September 22, 2022 21:25
@gmao-jkolassa
Copy link
Contributor

This is just to confirm that an ensemble experiment using the current version of this branch works without any issues for both CatchCNCLM40 and CatchCNCLM45. Note that I only tested that it runs. Please let me know if you would like a full science-validation of this.

weiyuan-jiang and others added 5 commits September 28, 2022 13:06
- bug fix in format string in get_ensid_string()
- replaced variable "id_string" with "ensid_string" for clarity
- cleanup and additional clarification of 3-digit ensid string for ensemble met forcing
@weiyuan-jiang
Copy link
Contributor Author

We have to change MAPL if we don't add 2 to the ens_id_with. I think the id should include "_exxxx" even we drop of the backward compatibility. @gmao-rreichle

@weiyuan-jiang
Copy link
Contributor Author

weiyuan-jiang commented Oct 6, 2022

@gmao-rreichle @gmao-jkolassa It should work now. But the nightly test would fail because we have hard coded HISTORY.rc for global assim and globlecs assim

@gmao-rreichle
Copy link
Contributor

We have to change MAPL if we don't add 2 to the ens_id_with. I think the id should include "_exxxx" even we drop of the backward compatibility. @gmao-rreichle

@weiyuan-jiang, I don't understand how MAPL comes into this. I agree that we don't want to change MAPL, but maybe there's another way to distinguish between the number of digits and the total string length (ens_id_width) in LDAS.

@weiyuan-jiang
Copy link
Contributor Author

These two assert should ease your concern if the users still use 4 for the width. Actually, the 6 is used in the lads_setup which should not have problem.
](

_ASSERT( ens_id_width < 12, "need 1 billion ensemble members? increase ens_id_width first")
_ASSERT( ens_id_width >= 5, "need at least 5 (_exxx) for hard coded ensemble forcing ( 3, xxx)")
)

@weiyuan-jiang
Copy link
Contributor Author

weiyuan-jiang commented Oct 6, 2022

The way we extract the template has to change in MAPL if we change the meaning of width.
https://github.com/GEOS-ESM/MAPL/blob/160a8d720f5357d07d419818db7f53d51d471f2b/generic/MAPL_Generic.F90#L1508-L1509

@gmao-jkolassa
Copy link
Contributor

I ran two shorts tests with the latest version of this branch. It builds successfully and runs successfully for both CatchCN40 and CatchCN45.

@weiyuan-jiang
Copy link
Contributor Author

When this branch is merged, the two files for nightly test should be changed too:
/discover/nobackup/mathomp4/LDAS_Restarts/NGHTLY_TST_TV4000/HISTORY_L4_SM.rc
/discover/nobackup/mathomp4/LDAS_Restarts/NGHTLY_TST_CS/assim/ldas179b8_C180/run/HISTORY.rc

replace VEGDYN0000 with VEGDYN_e0000

@mathomp4 , We will let you know when we merge

@gmao-rreichle
Copy link
Contributor

gmao-rreichle commented Oct 6, 2022

@weiyuan-jiang, @gmao-jkolassa, I applied a hopefully final set of minimal edits f1bd407
I don't think we need to retest the new functionality.
@weiyuan-jiang, did you run the LDAS Nightly tests with the above-mentioned edits to HISTORY.rc? If we are sure the GEOSldas PR is 0-diff for GEOSldas, we can ask @biljanaorescanin to test the matching GCM GC PR GEOS-ESM/GEOSgcm_GridComp#645 and then ask Scott to merge the matching GCM GC PR as soon as he can.
[I inadvertently closed the PR and then reopened it.]

@weiyuan-jiang
Copy link
Contributor Author

weiyuan-jiang commented Oct 7, 2022

it passed the test.

@gmao-rreichle gmao-rreichle marked this pull request as ready for review October 18, 2022 16:52
@gmao-rreichle gmao-rreichle merged commit 9a7ec25 into develop Oct 18, 2022
@gmao-rreichle gmao-rreichle deleted the feature/wjiang/catchcn_ensemble_new branch October 18, 2022 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants