Skip to content
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

use of mct_mod is deprecated #2294

Open
18 tasks done
jedwards4b opened this issue Dec 15, 2023 · 20 comments · Fixed by #2853
Open
18 tasks done

use of mct_mod is deprecated #2294

jedwards4b opened this issue Dec 15, 2023 · 20 comments · Fixed by #2853
Assignees
Labels
code health improving internal code structure to make easier to maintain (sustainability) done Issues whose closing PR is done but not yet merged (pending test re-run ok)

Comments

@jedwards4b
Copy link
Contributor

jedwards4b commented Dec 15, 2023

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

Definition of done:

  • Remove need for MCT in Lilac
  • Remove MCT in README documentation
  • Remove MCT from our standard testing
  • Remove MCT and CPL7 from Externals.cfg
  • Remove MCT specific directories from .config_files.xml
  • Remove MCT option from CLMBuildNamelist.pm and build-namelist and unit tester
  • Remove from bld/config_files/config_definition_ctsm.xml and bld/config_files/config_definition_ctsm.xm
  • Remove from python/ctsm/modify_input_files/fsurdat_modifier.py
  • Remove components/clm/src/unit_test_stubs/csm_share/CMakeLists.txt
  • Remove components/clm/src/unit_test_stubs/csm_share/mct_mod_stub.F90
  • Remove from User's guide
  • Remove from test/tools/test_driver.sh
  • Remove the src/cpl/mct directory
  • The mct subroutines in src/main/glc2lndMod.F90

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

  • Update PIO and any other CESM externals needed to fully remove MCT
  • Remove mct from .gitmodules
  • Remove MCTID from src/cpl/nuopc/lnd_comp_nuopc.F90 (this is just the component ID, so the name doesn't matter)
  • Remove mct comp_interface option from build-namelist, abort on error if set
@jedwards4b
Copy link
Contributor Author

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

@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Dec 15, 2023
@ekluzek ekluzek added this to the CESM3 milestone Dec 15, 2023
@samsrabin samsrabin added the code health improving internal code structure to make easier to maintain (sustainability) label Feb 15, 2024
@ekluzek ekluzek removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Feb 15, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Feb 15, 2024

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
Copy link
Contributor Author

@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
Copy link
Contributor

wwieder commented Feb 15, 2024

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
Copy link
Contributor Author

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

@ekluzek
Copy link
Collaborator

ekluzek commented Feb 15, 2024

@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
Copy link
Member

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
Copy link
Contributor

dlawrenncar commented Feb 16, 2024 via email

@jedwards4b
Copy link
Contributor Author

@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
Copy link
Collaborator

ekluzek commented Feb 16, 2024

@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
Copy link
Member

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 b4d64ac); 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
Copy link
Collaborator

ekluzek commented Feb 16, 2024

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

@billsacks
Copy link
Member

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
Copy link
Collaborator

ekluzek commented Feb 16, 2024

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
Copy link
Contributor

dlawrenncar commented Feb 16, 2024 via email

@billsacks
Copy link
Member

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 😆

@wwieder wwieder removed the CESM3 label Mar 6, 2024
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 18, 2024
@samsrabin
Copy link
Collaborator

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

@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 21, 2024
@ekluzek ekluzek modified the milestones: CESM3, cesm2_3_beta18 Apr 26, 2024
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Apr 26, 2024
@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label May 9, 2024
@samsrabin
Copy link
Collaborator

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

@ekluzek
Copy link
Collaborator

ekluzek commented May 15, 2024

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
Copy link
Contributor

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.

@wwieder wwieder moved this to Back Burner (or lower priority) in CTSM-CLM6 development highlights Jun 17, 2024
@ekluzek ekluzek moved this from Back Burner (or lower priority) to Prep line - not close to the oven in CTSM-CLM6 development highlights Jul 25, 2024
@ekluzek ekluzek linked a pull request Nov 1, 2024 that will close this issue
@ekluzek ekluzek added the done Issues whose closing PR is done but not yet merged (pending test re-run ok) label Nov 1, 2024
@ekluzek ekluzek removed their assignment Nov 5, 2024
@slevis-lmwg slevis-lmwg assigned ekluzek and unassigned slevis-lmwg Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health improving internal code structure to make easier to maintain (sustainability) done Issues whose closing PR is done but not yet merged (pending test re-run ok)
Projects
Status: On the grill (work in progress)
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants