search

ESCOMP / CTSM

Community Terrestrial Systems Model (includes the Community Land Model of CESM)
http://www.cesm.ucar.edu/models/cesm2.0/land/
Other
310 stars 315 forks source link

use of mct_mod is deprecated #2294

Open jedwards4b opened 11 months ago

jedwards4b commented 11 months ago

And must be removed prior to CESM3. https://github.com/ESCOMP/CTSM/blob/master/lilac/src/lilac_mod.F90#L11

Definition of done:

Final steps to do after PIO is updated and CESM is ready for the final removal

jedwards4b commented 11 months ago

Also components/clm/src/unit_test_stubs/csm_share/CMakeLists.txt components/clm/src/unit_test_stubs/csm_share/mct_mod_stub.F90

ekluzek commented 9 months ago

Discussed in meeting, this is important to do. Really want this done before CESM3, and it's in our planning now for that. Right now we don't have someone to work on this.

jedwards4b commented 9 months ago

@ekluzek As I understand it it is no longer needed in ctsm, or mosart and we have removed it from csm_share. If you do not remove it from slim prior to cesm3 then I propose that slim is not released with cesm3. What other requirement is there for mct?

wwieder commented 9 months ago

I think this issue is about LILAC, not SLIM, but maybe I'm wrong.
Maybe it's worth hearing from @dlawrenncar what the plan for LILAC should be moving forward?

jedwards4b commented 9 months ago

@wwieder Thanks for the clarification - it looks like lilac is a single processor application - am I reading that correctly?

ekluzek commented 9 months ago

@jedwards4b LILAC is not just a single processor app. LILAC is about coupling CTSM to other atmospheric models rather than through CESM. So coupling to WRF for example. So it'll use multiple processors just as you would with CTSM or WRF on their own.

It looks like MCT is being used for CPL7-data-models for LILAC to fill the roll of a DATM for the testing. ESMF is widely used in LILAC, and the CTSM cap for LILAC is an ESMF component, so it shouldn't be too hard to do this, but it's not trivial either. I think when LILAC was developed CDEPS wasn't mature enough to use it yet. Now we just need to switch the use of CPL7/DATM to CDEPS/DATM.

We don't currently have experts in LILAC right now. @billsacks is on ESMF, but maybe we could get some consulting time from him? And Negin is in CISL now. So we could only ask her as a favor for us. Right now we don't have someone to work on this, but hopefully can before CESM3 release. And @wwieder is right we need to assess the importance of supporting LILAC for going forward.

billsacks commented 9 months ago

It looks like MCT is being used for CPL7-data-models for LILAC to fill the roll of a DATM for the testing. ESMF is widely used in LILAC, and the CTSM cap for LILAC is an ESMF component, so it shouldn't be too hard to do this, but it's not trivial either. I think when LILAC was developed CDEPS wasn't mature enough to use it yet. Now we just need to switch the use of CPL7/DATM to CDEPS/DATM.

Thanks a lot for your investigation of this @ekluzek . I think this is mostly right, but a minor correction is that I think the use of MCT / data model functionality is to fill in fields that might be missing from the actual atmosphere. Currently this is used for prescribing aerosols. I agree that the right path forward is to change lilac_atmaero.F90 to use CDEPS streams.

(LILAC also has a fake atmosphere driver, lilac/atm_driver/atm_driver.F90, which serves a similar purpose as DATM for testing, though much simpler. But I don't think that relies on MCT: it just uses hard-coded data.)

dlawrenncar commented 9 months ago

I am hesitant to keep adding to SE burdens, but having a regional model capability, coupled to CTSM is an important feature. That could be through LILAC or it could be if WRF was coupled into CESM infrastructure ... or it could be if MPAS was coupled into CESM with regional modeling capability (not working, right?) ... or it could be possibly even be MPAS coupled to CESM with regionally-refined grid and atmosphere nudging outside the regionally-refined domain. My understanding is that the last option should be working now (?), though I don't really know the relative costs and whether or not this is really a good way to have regional modeling capability. This might be part of a bigger CESM Project discussion on regional modeling and what we want to and/or can support.

On Thu, Feb 15, 2024 at 2:11 PM Bill Sacks @.***> wrote:

It looks like MCT is being used for CPL7-data-models for LILAC to fill the roll of a DATM for the testing. ESMF is widely used in LILAC, and the CTSM cap for LILAC is an ESMF component, so it shouldn't be too hard to do this, but it's not trivial either. I think when LILAC was developed CDEPS wasn't mature enough to use it yet. Now we just need to switch the use of CPL7/DATM to CDEPS/DATM.

Thanks a lot for your investigation of this @ekluzek https://github.com/ekluzek . I think this is mostly right, but a minor correction is that I think the use of MCT / data model functionality is to fill in fields that might be missing from the actual atmosphere. Currently this is used for prescribing aerosols. I agree that the right path forward is to change lilac_atmaero.F90 to use CDEPS streams.

(LILAC also has a fake atmosphere driver, lilac/atm_driver/atm_driver.F90, which serves a similar purpose as DATM for testing, though much simpler. But I don't think that relies on MCT: it just uses hard-coded data.)

— Reply to this email directly, view it on GitHub https://github.com/ESCOMP/CTSM/issues/2294#issuecomment-1947345731, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFABYVACZOQTVMYDPVH5DFTYTZ2ZDAVCNFSM6AAAAABAW3SFJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBXGM2DKNZTGE . You are receiving this because you were mentioned.Message ID: @.***>

jedwards4b commented 9 months ago

@dlawrenncar I agree that regional modeling should be part of the longer term plan, but do we need it for cesm3? LILAC has a dependency on mct - a library that cseg has long slated for removal in cesm3. As far as I can tell it's the last place that is dependent on mct - if we leave mct in cesm3 we are on the hook to support it for many years to come - I think LILAC could be updated to have that dependency removed with fairly small effort, but the ctsm group is saying that LILAC may not be part of the longer term plan and maybe it should just be removed altogether. So I am looking for a response to a very specific question - update LILAC to remove the dependence on mct or remove LILAC from the cesm3 release. Doing nothing and releasing cesm3 with an mct dependence is not an option I want to consider.

ekluzek commented 9 months ago

@dlawrenncar can we make a commitment that removing MCT is a requirement of the CESM3 release? I think @jedwards4b is right about this in terms of future support. If we make that commitment that we will hold up the release for anything requires MCT. So for LILAC that means either updating it, or removing it. The other linchpin is SLIM, and I'd like to say that we hold up the release until it's updated to not require MCT as well.

I'm pretty sure we could get agreement in both CSEG and SEWG that the CESM3 release should require the removal of MCT. If we can have that commitment we don't have to stress about this. @dlawrenncar should @jedwards4b make the case to the CESM project meeting in this regard? Or it could be @briandobbins as well.

billsacks commented 9 months ago

I just looked a little more carefully at the mct uses in LILAC. I actually think that these are unused references at this point: @mvertens already converted the LILAC streams code to use CDEPS (in b4d64ac56); it just looks like the uses of MCT in lilac_mod weren't removed at that point, but it looks like they can be. I'll try removing them and running the LILAC test. It looks like the LILAC test covers the usage of the LILAC atmaero stream module, so if this passes and is bfb I think we can feel safe in removing this. I'll reply soon with my result.

ekluzek commented 9 months ago

Oh, awesome @billsacks thanks for looking into this! That is a better than expected answer! Hopefully, your guess will work.

billsacks commented 9 months ago

Yes, removing MCT references from LILAC was simple. While I'm at it, I'm trying to remove the unit test references to mct. Then I'll open a PR with those changes.

@jedwards4b thanks a lot for pointing us to these mct references that need to be removed!

ekluzek commented 9 months ago

You rock @billsacks! Looking forward to seeing that PR. You should probably make the PR to go to the new b4b-dev branch rather than master, as from a testing perspective I would think this would be bit-for-bit. If it's not it should go to master though...

dlawrenncar commented 9 months ago

This is good news. I think, then, that we should probably go ahead and plan not to support MCT in the release. It sounds like it is just SLIM that wouldn't currently work and I think we can talk about where getting SLIM working for CESM3 fits into the priority list. It's definitely a want, but whether or not it is a need is a little less clear. This is also something (SLIM) that could occur after the science freeze, I think, since it will not impact the default simulation.

On Fri, Feb 16, 2024 at 12:15 PM Erik Kluzek @.***> wrote:

You rock @billsacks https://github.com/billsacks! Looking forward to seeing that PR. You should probably make the PR to go to the new b4b-dev branch rather than master, as from a testing perspective I would think this would be bit-for-bit. If it's not it should go to master though...

— Reply to this email directly, view it on GitHub https://github.com/ESCOMP/CTSM/issues/2294#issuecomment-1949163686, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFABYVAMAWL6SBOHDSSXLHLYT6V6PAVCNFSM6AAAAABAW3SFJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBZGE3DGNRYGY . You are receiving this because you were mentioned.Message ID: @.***>

billsacks commented 9 months ago

You rock @billsacks! Looking forward to seeing that PR. You should probably make the PR to go to the new b4b-dev branch rather than master, as from a testing perspective I would think this would be bit-for-bit. If it's not it should go to master though...

Well, I appreciate it, but I think the only thing I really get credit for here is an ability to spot some low-hanging fruit :satisfied:

samsrabin commented 8 months ago

SE meeting today: @ekluzek will do most of this, with @slevis-lmwg also helping. Targeting beta 18.

samsrabin commented 6 months ago

SE meeting today: Will do this as part of #2493.

ekluzek commented 6 months ago

For LILAC There is lilac/CMakeLists.txt has this line

add_subdirectory(${CIME_ROOT}/../components/cpl7/driver drv_share)

Which I think needs to point to components/cmeps/cesm/nuopc_cap_share. To find the NUOPC version of glc_elevclass_mod.F90. There might also be code changes that go along with using the NUOPC version rather than the MCT version.

slevis-lmwg commented 6 months ago

For LILAC There is lilac/CMakeLists.txt has this line

add_subdirectory(${CIME_ROOT}/../components/cpl7/driver drv_share)

Note that the above turned out to be a "red herring" and according to Bill Sacks the /lilac/CMakeLists.txt can be removed.