ESCOMP / MOSART

Model for Scale Adaptive River Transport, Mosart, part of the Community Earth System Model
http://www.cesm.ucar.edu/
Other
8 stars 27 forks source link

major mosart refactor including addition of new halo capability #76

Closed mvertens closed 4 months ago

mvertens commented 9 months ago

Removed all references to rtm

New modularity:

New halo capability


Issues addressed:
Fixes #93 
Fixes #98 
Fixes #97
Fixes #99

Testing on derecho:
- Verified that the following tests passed and were bfb with master
  -   ERP_Ld3.f19_f19_mtn14.2000_DATM%GSWP3v1_CLM50%SP_SICE_SOCN_MOSART_SGLC_SWAV 
  -   SMS_D_Ln5.f19_f19_mtn14.2000_DATM%GSWP3v1_CLM50%SP_SICE_SOCN_MOSART_SGLC_SWAV
   -  In both of the above use_halo_option was set to .true. in user_nl_mosart.
- Verified that answers did not change with respect to mosart1_0_49 
  -  generated baselines xml-category mosart using cesm2_3_beta17 (baseline directory is called mosart1_0_49)
  - modified cesm2_3_beta17 to use mosart branch feature/refactor and reran the mosart tests - all results were bfb
ekluzek commented 9 months ago

@mvertens can you tell us more about the new capability that is planned to MOSART after this refactor. For CESM4 we are hoping that we can deprecate and eliminate the use of MOSART in CESM and ONLY use mizuRoute.

A big limitation of MOSART is that we don't have tools to make new input datasets nor people we can rely on to make the datasets. This means MOSART can NOT be used for Paleo work for example, which kills it's usage for CESM4. At NCAR we don't have local experts for MOSART only for mizuRoute. Removing RTM and MOSART for CESM4 would allow us to only support one river model for CESM4 which would lower the projects maintenance costs.

Have you looked into adding these same changes to mizuRoute instead of (or in addition to) MOSART? We should at least look into that.

mvertens commented 9 months ago

@ekluzek - the key capability that was needed is halo updates. This is currently needed by @swensosc for his work and he was targeting MOSART. I think doing this in MOSART was very easy and MOSART is the code base that will be supported in both NorESM3 and CESM3. So having a clean refactored code will be helpful to both efforts in my opinion. As for MizuRoute - I'm happy to talk about that in the future.

ekluzek commented 9 months ago

I think this is a big enough of a change that we should bump the middle number in the version sequence. And we might as well make the name more similar to CTSM tags.

So this becomes

mosart1.1.dev01

jedwards4b commented 9 months ago

I have to say - I don't like the 'dev' part. Just make it mosart1.1.01

ekluzek commented 8 months ago

In talking about this today, there's one bit of additional functionality that will be added (having to do with gradients). This tag will be the basis for the GW branch, as well as the DOM work that is being done at NIO.

We also decided that this would be done after the list of simple PR's that will be done first.

mvertens commented 4 months ago

@slevis-lmwg @ekluzek @jedwards4b - I have verified that this branch does not change answers with respect to mosart1_0_49. @mvdebolskiy also participated in updating this refactor branch to have updated internal diagnostics. This branch will be the starting point of the new glc->rof coupling that will replace glc->ocn.

ekluzek commented 4 months ago

The plan right now is for @slevis-lmwg to bring this in after he has some time clear with bringing CN Matrix to CTSM.

slevis-lmwg commented 4 months ago

OK derecho test-suite FAIL izumi test-suite during build; I have restarted the build in case it was a glitch; see here in case anyone wishes to take a look before Erik and I get to it: /fs/cgd/data0/slevis/git_externals/major_refactor/tests_0605-174443iz

mvertens commented 4 months ago

@slevis-lmwg - there were indeed compilation problem with nag for this PR. I have fixed the problems and confirmed that mosart can now build with nag on izumi. I have pushed the changes back.

mvertens commented 4 months ago

@slevis-lmwg - I have addressed #93 and pushed the changes back. It ends up I had to revert these changes since adding the PIO_BCAST_ERROR call caused the model to crash with a segmentation fault. @jedwards4b - can you please help with this. @slevis-lmwg @jedwards4b - I think I know the problem. You don't get an file descriptor until you actually create or open the file. So you cannot call PIO_BCAST_ERROR before those calls - since that doesn't make sense. @slevis-lmwg - I have now fixed these issues based on input from @jedwards4b .

ekluzek commented 4 months ago

Awesome, thanks for working on that @mvertens and being able to get it done quickly. We opened the issue in case this was a longer term thing that didn't need to be handled now. We figured it wasn't required for the Science capability freeze timeline, so a longer term issue also made sense. But, even better that you've already resolved it.

slevis-lmwg commented 4 months ago

I reran the mosart test-suite: OK on izumi, only NLCOMP diffs OK on derecho, only NLCOMP and an EXPECTED FAILURE

UPDATE: I reran the izumi test-suite for sanity, because I pushed one more commit after the last testing.

jedwards4b commented 4 months ago

@slevis-lmwg This repository does not have automated tag creation - will you make a new tag?

slevis-lmwg commented 4 months ago

I type git tag from my latest master (local), and I get mosart1.1.01 which is the annotated that I created and pushed to escomp.

On github I see the same tag here: https://github.com/ESCOMP/MOSART/tags

Please let me know if I'm not answering your question.

jedwards4b commented 4 months ago

I think that you must have made the tag shortly after I posted that message - thanks.