Skip to content

Add WW3 daily-averaged wave fields needed for the NorESM merge#683

Open
alperaltuntas wants to merge 3 commits into
ESCOMP:mainfrom
alperaltuntas:ww3_aux_hist_fields
Open

Add WW3 daily-averaged wave fields needed for the NorESM merge#683
alperaltuntas wants to merge 3 commits into
ESCOMP:mainfrom
alperaltuntas:ww3_aux_hist_fields

Conversation

@alperaltuntas

Copy link
Copy Markdown
Member

Description of changes

This PR is needed to sync WW3 forks of CESM and NorESM. The NorESM WW3 cap exports 11 new Sw_*_avg fields and expects the mediator to output them in a daily-averaged auxiliary history file. This defines those fields in fd_cesm.yaml, and drops the stale ww3 histaux overrides that were still requesting the old instantaneous fields.

Specific notes

This PR must be evaluated and merged in conjunction with:

No answer changes

The only user interface changes is the default value and description of histaux_wav2med_file1_doavg.

Testing performed

aux_ww3

The NorESM WW3 cap exports 11 new Sw_*_avg fields and expects the mediator
to output them in a daily-averaged auxiliary history file. This defines
those fields in fd_cesm.yaml, and drops the stale ww3 histaux overrides
that were still requesting the old instantaneous fields.

@billsacks billsacks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mvertens recently made some changes to the ww3 aux hist fields - see #677. It looks like this PR may need to be updated to merge cleanly with that one... but I also wonder if the changes from @mvertens make some of this unnecessary???

Once you reconcile this with @mvertens 's changes, I'm fine with this PR as long as @mvertens is good with it - so no need for further review from me.

@mvertens

Copy link
Copy Markdown
Collaborator

@billsacks - I am wondering actually why none of the latest CMEPS changes that I brought in made it into this PR. But that said - those changes become irrelevant if the noresm defaults are picked up - which looking at this PR seems to be the case. We no longer need the noresm/cesm distinction - since only the noresm one is kept.
@alperaltuntas - I am wondering why the differences with cmeps main did not show up in the PR differences. I accepted the PR, however, given the fact that we just want the noresm defaults.

@alperaltuntas

Copy link
Copy Markdown
Member Author

@mvertens I started off of cmeps1.1.48 but then merged the latest main before submitting the PR, so I am not sure why your changes don't show up. Happy to revise it if you have a suggestion, but you are right that we are adopting NorESM defaults, so we probably no longer need noresm/cesm distinction (at least for the time being).

@alperaltuntas

Copy link
Copy Markdown
Member Author

@mvertens @billsacks I just realized I previously merged a stale main, so that's why @mvertens's latest changes didn't show up. I merged with the latest main this time, and will run a few tests to double check things.

@mvertens

Copy link
Copy Markdown
Collaborator

@alperaltuntas - thanks for clarifying - this all makes sense. I'm happy to review again.

@alperaltuntas

Copy link
Copy Markdown
Member Author

Update: with the latest merge commit, all of my tests passed. So, this PR is ready again for review.

@billsacks

Copy link
Copy Markdown
Member

Thanks, @alperaltuntas . I'm going to remove myself as a reviewer, deferring to you and @mvertens on this. It seems like you could potentially remove the coupling_mode selector from histaux_wav2med_file1_flds if those are back to being the same for cesm & noresm, but I'll let you and @mvertens make that decision.

@mvertens mvertens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@alperaltuntas - thanks. This looks correct to me now.

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.

3 participants