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

Make sure to define state_vector if not in land use mode #1223

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

glemieux
Copy link
Contributor

@glemieux glemieux commented Jul 15, 2024

Description:

This is a bug fix for #1221 which came in with the latest tag sci.1.77.0_api.36.0.0. The local variable state_vector is undefined in disturbance_rates if land use mode is off. To avoid indexing into an array of NaN we need to define values for state_vector. The fix proposed here is to set it by getting the current state vector, which is called at the top of the subroutine.

Collaborators:

Expectation of Answer Changes:

Answer changes are not expected

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Documentation

Test Results:

CTSM (or) E3SM (specify which) test hash-tag: 12c60f3de477bbdaa2cf7a8afe0d089886d2c960

CTSM (or) E3SM (specify which) baseline hash-tag: 12c60f3de477bbdaa2cf7a8afe0d089886d2c960

FATES baseline hash-tag: sci.1.77.0_api.36.0.0

Test Output: TBD

@glemieux glemieux changed the title Make sure to define state_vector if not in land use mode Make sure to define state_vector if not in land use mode Jul 15, 2024
@glemieux glemieux requested a review from ckoven July 15, 2024 16:46
@glemieux glemieux linked an issue Jul 15, 2024 that may be closed by this pull request
Copy link
Contributor

@ckoven ckoven left a comment

Choose a reason for hiding this comment

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

this makes sense to me, thanks @glemieux.

@glemieux
Copy link
Contributor Author

Testing the previously failing tests on perlmutter results in those tests passing now.

Location: /pscratch/sd/g/glemieux/e3sm-tests/pr6353-fates_lu-nanfix-fld.fates.pm-cpu..E6d21ce5657-Fa2b71d0d

Waiting for derecho to come back up to access izumi tests results.

@glemieux
Copy link
Contributor Author

Izumi testing of the failed case now passes.

Location: /scratch/cluster/glemieux/ctsm-tests/tests_0717-114533iz

@glemieux
Copy link
Contributor Author

Regression testing against the PR baseline generated for derecho is b4b for all expected tests:

Location: /glade/derecho/scratch/glemieux/ctsm-tests/tests_pr1223-fates

@glemieux glemieux merged commit 1982b00 into NGEET:main Jul 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

state_vector is NaN when land use is off
2 participants