search

ESCOMP / CDEPS

Community Data Models for Earth Prediction Systems
https://escomp.github.io/CDEPS/versions/master/html/index.html
20 stars 45 forks source link

Bring support for configurable source and destination mask #259

Closed uturuncoglu closed 8 months ago

uturuncoglu commented 9 months ago

Description of changes

This PR aims to bring feature of changing source and destination masks via configuration file (ESMF config). This is especially important for the CDEPS inline capability since we have no control in destination mask.

Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial): No

Any User Interface Changes (namelist or namelist defaults changes): The two new optional namelist option is introduced to the ESMF config file: stream_src_mask and stream_dst_mask to control mask values. The deafest are zero which does not change the existing behavior. Additional code needs to be implemented if we want to support XML configuration files.

Testing performed (e.g. aux_cdeps, CESM prealpha, etc): This is initially tested under UFS Weather Model and the modifications did not change the answer UFS Weather Model. CESM still need to be tested.

Hashes used for testing:

uturuncoglu commented 9 months ago

@jedwards4b I still need to test it with CESM. The UFS Weather Model is fine with these changes and I did not see any answer change. I think I need to test this on Derecho. Is there anything specific that I need to know (i.e. the location of baseline, which baseline needs to be used with which CESM version).

uturuncoglu commented 9 months ago

@jedwards4b BTW, let me know if you would like to support XML configuration way. I am not sure we could test those changes under CESM since we need a case like HAFS configuration.

jedwards4b commented 9 months ago

As long as this method is neither required nor needed for cesm I don't think we need to add support for it yet.

uturuncoglu commented 9 months ago

@jedwards4b Okay. That is fine. Thanks. Let me test with CESM before merging this PR. I'll update you soon.

uturuncoglu commented 8 months ago

@jedwards4b @mvertens I am still working on CMEPS side and trying to test with regional MOM6. This is independent from the CMEPS PR but maybe it would be nice to wait to marge this until we completed testing of CMEPS.

uturuncoglu commented 8 months ago

@jedwards4b Do you know any test that uses CDEPS inline? I would like to run one of them to see if there is an issue. I am still investigating CMEPS issue but if there is no any issue with CDEPS we could merge it.

jedwards4b commented 8 months ago

I think that some of the component models use CDEPS inline in the B compset, so you've already done this test.

uturuncoglu commented 8 months ago

@jedwards4b like ERR_Vnuopc_Ld5.f09_t061.B1850MOM.derecho_intel.allactive-defaultio?

uturuncoglu commented 8 months ago

@jedwards4b Okay. The land component is using CDEPS inline in ERR_Vnuopc_Ld5.f09_t061.B1850MOM.derecho_intel.allactive-defaultio test and it looks fine and run without any issue. I think we could merge this.