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

add this back for mct CLM_USRDAT #3954

Merged
merged 2 commits into from
May 11, 2021

Conversation

jedwards4b
Copy link
Contributor

Add back domain for CLM_USRDAT for mct driver.

Test suite: SMS_D_Lm1_Mmpi-serial.CLM_USRDAT.I1PtClm50SpRs.cheyenne_intel.clm-USUMB (All pass)
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #3905

User interface changes?:

Update gh-pages html (Y/N)?:

@jedwards4b jedwards4b requested a review from ekluzek May 5, 2021 19:03
Copy link
Contributor

@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.

We may need to chat about this. But, I actually do NOT want this to come in. We never used these generically named files and when I asked you to remove them I was just asking you to remove unused code. This isn't the problem that I'm seeing. I can't find the issue I made regarding this right now, but when I do I'll point it out.

@jedwards4b
Copy link
Contributor Author

@ekluzek in your issue #3905 the problem is that ATM_GRID and LND_GRID are unset - this is the code that causes them to be set. It's done in grids.py as part of create_newcase.

@ekluzek
Copy link
Contributor

ekluzek commented May 5, 2021

Ahhh, OK. You are right. Of course (as you often are). I was convinced you were wrong, but I had also tracked it down to _get_domains in grid.py which corresponds to the XML you are pointing out. So this does now totally make sense.

I really don't want the domain file set though, but it looks like the block doesn't need it as far as I can see at this point. But, that just means removing that one line. And I suggest adding a comment to say you have to set it by hand in the case with xmlchange.

Comment on lines 40 to 41
<grid name="glc" compset="SGLC" >null</grid>
<grid name="glc" compset="CISM2" >gland4</grid>
Copy link
Member

Choose a reason for hiding this comment

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

This change (while a good fix of whitespace) is going to cause a conflict with the changes in #3943 . Can you please back this out here and I'll fix the whitespace in that PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR has correct whitespace in it. If there is a conflict the issue should be addressed in #3943.

Copy link
Member

Choose a reason for hiding this comment

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

I have pushed a whitespace fix to #3943 . The problem is that, even with the same whitespace fix in that PR, merging these two branches will produce a merge conflict. Since resolving merge conflicts can be error-prone, I was asking if we can preemptively solve the problem by backing out the whitespace change here and just letting it be fixed when #3943 is merged. If you give me your okay, I'm happy to make that change myself. The end result will be the same, but the merge conflict will be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, go ahead

Copy link
Member

Choose a reason for hiding this comment

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

Done; thanks

Copy link
Contributor

@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.

Ok. Jim is right this is what's needed. And I don't see anything else needed in cime for the generic NUOPC case.

@jedwards4b jedwards4b merged commit f01e480 into ESMCI:master May 11, 2021
@jedwards4b jedwards4b deleted the fix_clm_usrdata_mct branch May 11, 2021 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USRDAT resolution issues for both MCT and NUOPC drivers
3 participants