Skip to content

Conversation

@samsrabin
Copy link
Member

@samsrabin samsrabin commented Jul 2, 2025

Description of changes

Increases filename length in many places from 256 (CL) to 512 (CX). This was necessary to support a new test that ended up generating very long file paths (see ESCOMP/CTSM#3292).

I'm planning to merge in the latest CDEPS tag once I get an initial review.

Specific notes

Contributors other than yourself, if any: None

CDEPS Issues Fixed:

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

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

Any User Interface Changes (namelist or namelist defaults changes): No

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):

One CTSM test that was previously failing due to long file paths now passes.

aux_cdeps tests all pass except two that aren't the result of this work:

Hashes used for testing: 0595593

  • Remove the DATAMODEL test from: dglc/cime_config/testdefs/testlist_dglc.xml
  • Test after merging latest CDEPS tag

@ekluzek ekluzek added enhancement New feature or request answers are bfb Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group labels Jul 7, 2025
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

Looks good. This is nicely straightforward.

@samsrabin samsrabin marked this pull request as draft July 7, 2025 17:35
@ekluzek ekluzek marked this pull request as ready for review July 7, 2025 17:52
@ekluzek
Copy link
Collaborator

ekluzek commented Jul 7, 2025

@jedwards4b and @billsacks this is ready to come in, after @samsrabin updates to the latest cdeps tag and redoes testing. Do you have any comments here? Or can he do that and then get this merged in afterwards? Let us know of changes you need, or other testing you'd like to have done.

Thanks

@ekluzek
Copy link
Collaborator

ekluzek commented Jul 7, 2025

@samsrabin in light of the answer for #344, remove the DATAMODELTEST from:

dglc/cime_config/testdefs/testlist_dglc.xml

@samsrabin
Copy link
Member Author

@ekluzek Removing that file would remove the failing SMS_Ld3.f19_g17_rx1.2000_SATM_SLND_SICE_SOCN_DROF%NYF_SGLC_SWAV.derecho_intel test, not the failing SMS_D_Ld3.f10_f10_ais8gris4_mg37.DATAMODELTEST.derecho_gnu that #344 is about.

@jedwards4b
Copy link
Contributor

You just want to remove the DATAMODELTEST tests.

@ekluzek
Copy link
Collaborator

ekluzek commented Jul 7, 2025

Yes, sorry I got it backwards.

Remove the DATAMODETEST from: dglc/cime_config/testdefs/testlist_dglc.xml

I've corrected it above.

@samsrabin
Copy link
Member Author

@ekluzek Done!

Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

Thanks for this change - basically looks good. I see that you changed nlfilename and streamfilename for some data models but not others. Should this be consistent? See also the one inline comment below.

@samsrabin
Copy link
Member Author

Good catches, @billsacks. Those are now addressed, and re-testing has aux_cdeps still passing (except the one remaining known-bad test).

Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

Looks good now - thanks for these changes that add more consistency!

@jedwards4b jedwards4b merged commit 2544107 into ESCOMP:main Jul 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

answers are bfb enhancement New feature or request Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aux_cdeps test for DATAMODELTEST compset fails

4 participants