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

ENSO updates #295

Merged
merged 1 commit into from Apr 1, 2020
Merged

ENSO updates #295

merged 1 commit into from Apr 1, 2020

Conversation

forsyth2
Copy link
Collaborator

@forsyth2 forsyth2 commented Mar 24, 2020

Resolves #292.

  • Update derivations for NET_FLUX_SRF and LHFLX.
  • Change code to use TS if SST is not available.
  • Update start_yr and end_yr parameter handling.

Copy link
Collaborator Author

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

@chengzhuzhang I still need to run/test this code, but are these changes what you had in mind?

acme_diags/driver/enso_diags_driver.py Outdated Show resolved Hide resolved
run.py Outdated Show resolved Hide resolved
acme_diags/derivations/acme.py Outdated Show resolved Hide resolved
acme_diags/parameter/enso_diags_parameter.py Outdated Show resolved Hide resolved
Copy link
Contributor

@chengzhuzhang chengzhuzhang left a comment

Choose a reason for hiding this comment

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

Looking good, the system tests need to be updated for parameters specifying year range.

acme_diags/parameter/enso_diags_parameter.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

@chengzhuzhang Ready for review. I tagged you on a few questions I had.

acme_diags/derivations/acme.py Outdated Show resolved Hide resolved
acme_diags/driver/enso_diags_driver.py Show resolved Hide resolved
test.cfg Outdated Show resolved Hide resolved
acme_diags/driver/enso_diags_driver.py Outdated Show resolved Hide resolved
tests/system/all_sets.cfg Outdated Show resolved Hide resolved
acme_diags/derivations/acme.py Show resolved Hide resolved
acme_diags/derivations/acme.py Outdated Show resolved Hide resolved
tests/system/all_sets.cfg Show resolved Hide resolved
tests/system/all_sets.cfg Show resolved Hide resolved
@chengzhuzhang
Copy link
Contributor

 'NET_FLUX_SRF': OrderedDict([
        (('FSNS','FLNS','LHFLX','SHFLX'), lambda fsns, flns, lhflx, shflx: netflux4(fsns, flns, lhflx, shflx)),
        (('rsds','rsus','rlds','rlus','hfls','hfss'), lambda rsds, rsus, rlds, rlus, hfls, hfss: netflux6(rsds, rsus, rlds, rlus, hfls, hfss)),

I meant to add an entry
(('FSNS','FLNS','QFLX','SHFLX'), lambda fsns, flns, lhflx, shflx: netflux4(fsns, flns, lhflx, shflx)),

with this netflux4 function needs to take in account the qflx to lflx conversion. Or we can have a new netflux4 function just for take care of this.

@forsyth2 forsyth2 marked this pull request as ready for review March 27, 2020 15:54
Copy link
Collaborator Author

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

@chengzhuzhang See the second commit for my changes. Please test again and see if that works for you. If so, I'll fixup the commits and merge.

I spent some time trying to add a test case, but that ran into complications. I wanted to add a test where the start years are unequal. However, it looks like we only have two years of data for TREFHT in the tests directory, and if you set one of the start years to equal the end year, then the code fails (this is true on master as well). I'm not sure if that's expected behavior or not -- in any case, that would be outside the scope of this pull request to address.

Copy link
Contributor

@chengzhuzhang chengzhuzhang left a comment

Choose a reason for hiding this comment

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

https://compy-dtn.pnl.gov/zhan429/enso_reg_diags_alpha22_to_obs/viewer/enso_diags/index.html

The regression map has succesfully generated, but scatter plots are missing. due to the AttributeError: 'EnsoDiagsParameter' object has no attribute 'start_yr' error..

acme_diags/driver/enso_diags_driver.py Show resolved Hide resolved
@forsyth2 forsyth2 merged commit 1ce04ce into E3SM-Project:master Apr 1, 2020
@forsyth2 forsyth2 deleted the enso-updates branch April 1, 2020 21:40
tomvothecoder pushed a commit to tomvothecoder/e3sm_diags that referenced this pull request Mar 23, 2021
tomvothecoder pushed a commit to tomvothecoder/e3sm_diags that referenced this pull request Mar 23, 2021
tomvothecoder pushed a commit to tomvothecoder/e3sm_diags that referenced this pull request Mar 24, 2021
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.

ENSO diags update
2 participants