-
Notifications
You must be signed in to change notification settings - Fork 371
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
Change MPAS-Ocean high-frequency output mode to append
#6434
Conversation
@zhangshixuan1987 and @wlin7, can you verify that this fixes #6432? I am happy to do some testing but I think you are in a better position to verify this. |
Note: an alternative fix would be to change: |
Hi @xylar: I conducted a set of 2-month short-term simulations on Compy with the code changes you suggested:
The changes here indeed solved the issues:
|
Those error messages are annoying for sure but ultimately harmless I think. Did you make both changes? compute/write on startup and append? If so, you only want append and not compute/write on startup. |
Do the high frequency files have all the expected time entries? |
Hi @xylar : Yes, I think that this change can solve the issues I originally encountered. However, when I employ this solution in a new G-case simulation, it seems to me that it can potentially trigger a new potential issue for the following scenario:
I think this error is expected from the purpose of |
Hi @xylar : Actually this is not the case. For the test simulation as I mentioned above, I ONLY made changes for https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-ocean/cime_config/buildnml#L862. No other changes have been made. |
@xylar: I think I overlooked the changes in this pull request. I thought that #6434 and #6432 were different. In fact, the pull request along with the changes here is indeed the correct one. To avoid confusion, I would like to clarify:
Thank you! |
I believe that append mode is functioning as I should, and that one simply has to ignore the disconcerting error messages if one is restarting from an earlier point and cannot overwrite the data. |
I think the append mode change remains the preferred solution despite the annoying error messages. In general, we cannot guarantee that fields selected for output are available at startup. Fields may contain all zeros. So it is safer to keep the results written before the last run wrote its final restart than to overwrite with new values after restart. You all may use the other solution of computing/writing at startup for the alternating B- and G-cases but that won't be what we ultimately do in E3SM. |
This prevents the last entry from the previous run before a restart from being clobbered by a new run.
0022636
to
1a5f91d
Compare
append
@wlin7, @zhangshixuan1987 and @mark-petersen, as discussed above, I'm going with the "alternative" method of fixing this issue where the output is in "append" mode rather than "truncate". This has the annoying side effect that error messages may appear after each restart (particularly if a run timed out) saying that entries could not be overwritten because the mode is "append". This was the preferred fix in our MPAS DevOps call last week. |
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.
Yes, I agree. The inconvenience of extra messages from a bad restart is preferred to having a time slice missing. Thanks for everyone's detailed review and testing above.
@mark-petersen, do you also agree that there's some risk in setting things to compute/write at startup? That worked for what @zhangshixuan1987 and @wlin7 need for their purposes but I think there are enough fields that start out as all zeros until after the first time step of a restart run, so that the solution I now have in this branch is preferable. But I would be keen to make sure you agree. |
@wlin7, I'm taking you off as a reviewer. I would still welcome your input if you want to provide it but I don't think it is necessary before we merge this. |
@jonbob please start merging this. |
Change MPAS-Ocean high-frequency output mode to append Without this, the first output is missing after each restart Fixes #6432 [BFB]
Passes:
merged to next |
merged to master |
Thank you, @jonbob! |
This merge updates the E3SM-Project submodule from [8939709](https://github.com/E3SM-Project/E3SM/tree/8939709) to [752f281b15](https://github.com/E3SM-Project/E3SM/tree/752f281b15). This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list): - [ ] (ocn) E3SM-Project/E3SM#6412 - [ ] (ocn) E3SM-Project/E3SM#6434 - [ ] (ocn) E3SM-Project/E3SM#6420
Without this, the first output is missing after each restart
fixes #6432