NGEET / fates

repository for the Functionally Assembled Terrestrial Ecosystem Simulator (FATES)
Other
105 stars 92 forks source link

Change all 'hite' to 'height' #1091

Closed JessicaNeedham closed 1 year ago

JessicaNeedham commented 1 year ago

This PR changes all occurrences of ‘hite’ in FATES to ‘height’. There are no changes for users since it was already correct in the parameter file and history outputs.

At the software meeting today there seemed to be some fond feelings towards this historical quirk but no strong objections to fixing it.

Collaborators:

Expectation of Answer Changes:

No changes to users. Should be bfb.

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

Integrator

Documentation

Test Results:

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

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

FATES baseline hash-tag:

Test Output:

Compiles and runs on Perlmutter CPU.

rosiealice commented 1 year ago

I am cool with this, although I will miss it slightly.

rgknox commented 1 year ago

I'm going to need some time sort through my feelings

aswann commented 1 year ago

This gives me flashbacks to some very frustrated times greping to figure out how height was calculated in ED2. It only took 13 years to change it!

rgknox commented 1 year ago

I think @aswann makes a compelling argument to change

ckoven commented 1 year ago

This seems great to me. One question is should we also change cohort%dbh to something like cohort%diameter or cohort%stem_diameter as well? DBH isn't always an accurate description of the variable (i.e. for grasses which use a basal diameter as the reference allometric variable).

glemieux commented 1 year ago

During regression testing a ran into a weird pio issue across all tests (including the one lone ctsm only testmod). I suspect this was a cheyenne glitch. I've double checked that the baseline runs fine. I'm running a single AllVars case again to confirm my suspicions.

glemieux commented 1 year ago

After fixing another hite variable that came in with deconflicting against the latest tags, I was able to successfully re-run the regression tests. All expected tests passed b4b.

cheyenne results: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr1091-hitefix