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

Adds support of DATM_MODE=CLM1PT for ELM #3845

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

bishtgautam
Copy link
Contributor

Modifications to support user-defined atmospheric forcing
with ELM.

Test suite:
Test baseline:
Test namelist changes:
Test status: [bit for bit, roundoff, climate changing]

Fixes [CIME Github issue #]

User interface changes?:

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

Code review:

Modifications to support user-defined atmospheric forcing
with ELM.
@bishtgautam
Copy link
Contributor Author

The new code in src/components/data_comps_mct/datm/cime_config/buildnml may not be an ideal solution, but I wanted to open this PR to get the conversation started on how to best accommodate these changes.

Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

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

I don't see any problems with this.

@bishtgautam
Copy link
Contributor Author

It seems I broke a test.

@jedwards4b
Copy link
Contributor

You did - do you know where to find the log? I also notice that you didn't run scripts_regression_tests.py locally for this PR. Can you please do that and update the initial comment of the conversation.

======================================================================
FAIL: test_pylint_src_components_data_comps_mct_datm_cime_config_buildnml (__main__.B_CheckCode)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./scripts_regression_tests.py", line 3480, in test
    self.assertTrue(result == "", msg=result)
AssertionError: False is not true : ************* Module buildnml
W: 57, 0: Bad indentation. Found 13 spaces, expected 12 (bad-indentation)
W: 59, 0: Bad indentation. Found 13 spaces, expected 12 (bad-indentation)
W: 65, 0: Bad indentation. Found 13 spaces, expected 12 (bad-indentation)
W: 67, 0: Bad indentation. Found 13 spaces, expected 12 (bad-indentation)

"A DATM_MODE for CLM is incompatible with DATM_TOPO=none.")
expect(grid != "CLM_USRDAT" or clm_usrdat_name in ("", "UNSET"),
"GRID=CLM_USRDAT and CLM_USRDAT_NAME is NOT set.")
if get_model() != "e3sm":
Copy link
Contributor

Choose a reason for hiding this comment

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

The code looks identical in both branches with the only difference being ELM vs CLM. You should capture this difference in a single string so the code can be reused.

Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

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

Better, thanks.

@rljacob
Copy link
Member

rljacob commented Feb 18, 2021

@jgfouca this should be merged if you're happy with it.

@jgfouca jgfouca merged commit 3f5a347 into master Feb 18, 2021
@jgfouca jgfouca deleted the bishtgautam/user-defined-forcing-for-elm branch February 18, 2021 20:12
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.

4 participants