tomvothecoder commented 5 months ago

Description

Closes #673

Todo

[x] Refactor aerosol_budget_driver.py
[x] Refactor aerosol_budget_plot.py
- [x] Add formulas to formulas.py
- [x] Add derived variables to derivations.py
Hybrid to pressure functionality
- [x] Add "hyai" and "hybi" to HYBRID_SIGMA_KEYS and expand accepted values
- [x] Add p0, a_key, and b_key to _hybrid_to_pressure()
- [x] Add logic to _hybrid_to_pressure() to update units based on p0
[x] Update call to get_name_yrs_attr() in aerosol_aeronet_driver.py
[x] Add changes from #804 and #805

5/8/24 Fixes

[x] Fix Elevated Emission (Tg/yr) -- much larger on dev -- due to using xarray .sum() instead of Python sum() on a tuple of variables
[x] Fix Burden -- units seem smaller on dev (unit conversion?) -- fixed, I think fixing derivation formulas fixed this
[x] Fix Lifetime (Days) -- uses Burden for calculation
[x] Fix Sulfate Elevated Emission (Tg/yr) -- due to sum(x[0]) on derived variable with wildcards, which can actually have more than 1 element -- fixed by updating derived var function to expect a list of N variables and perform sum() within the function
[x] Fix large differences in burden and lifetime values -- different units still? -- the old main results did not have the fixes from PR #805, while this branch did. I re-ran the 2.90 example script again which produced the latest aerosol_budget metrics that align with this branch (refer to results here)

Checklist

[ ] My code follows the style guidelines of this project
[ ] I have performed a self-review of my own code
[ ] My changes generate no new warnings
[ ] Any dependent changes have been merged and published in downstream modules

If applicable:

[ ] New and existing unit tests pass with my changes (locally and CI/CD build)
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

tomvothecoder commented 4 months ago

804 and #805 is related to this PR. We need to carry over any code changes here.

chengzhuzhang commented 4 months ago

Yes, this is in earlier code. It still is concerning (though I don't think it is wrong or blocking to have this behavior). We can add a fix to avoid these errors, I would have hoped the logic wouldn't reach the units check in these cases...

Agreed. I think I commented on the wrong PR, the conversation should go to https://github.com/E3SM-Project/e3sm_diags/pull/805.. Sorry, Tom.

tomvothecoder commented 4 months ago

Agreed. I think I commented on the wrong PR, the conversation should go to #805.. Sorry, Tom.

No problem @chengzhuzhang. I moved the comments over to that PR and deleted them here.

tomvothecoder commented 3 months ago

Viewer

CSV File Diff Comparison (VS Code)

"ANN" comparison -- no highlighted diffs

"JJA" comparison -- no highlighted diffs

E3SM-Project / e3sm_diags

CDAT Migration: Refactor `aerosol_budget` #800