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

Aerosol fix #303

Merged
merged 16 commits into from
Jun 24, 2024
Merged

Aerosol fix #303

merged 16 commits into from
Jun 24, 2024

Conversation

justin-richling
Copy link
Collaborator

@justin-richling justin-richling commented May 15, 2024

With the update of how some variables are derived if using a CAM-CHEM run, the derivation of time series files has been updated to check first if the constituents are in the history file. If not, it checks again for the CAM-CHEM constituents based off the argument derivable_from_cam_chem in the adf_variable_defaults.yaml file.

Additionally, this will add debugging if missing constituents to the debug log and update the CSS to force the number of plot boxes on the website from 3 to 4 on each row; 4 boxes was making the content go off page.

Add new argument in variable defaults file for constituents of variables that have changed in CAM-CHEM runs

Implement check in derivation of time series files to first check if chem/aerosol constituents are in history file then check if this variable is in the list of CAM-CHEM
@justin-richling justin-richling requested a review from nusbaume May 15, 2024 22:46
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Looks good! I do have some (hopefully easy) change requests, but none of them are major enough to require a re-review from me. Thanks!

lib/adf_base.py Outdated Show resolved Hide resolved
lib/adf_base.py Outdated Show resolved Hide resolved
lib/adf_diag.py Outdated Show resolved Hide resolved
lib/adf_info.py Outdated Show resolved Hide resolved
lib/adf_variable_defaults.yaml Outdated Show resolved Hide resolved
lib/test/unit_tests/test_adf_base.py Outdated Show resolved Hide resolved
lib/test/unit_tests/test_adf_base.py Outdated Show resolved Hide resolved
lib/test/unit_tests/test_adf_base.py Outdated Show resolved Hide resolved
@justin-richling
Copy link
Collaborator Author

@nusbaume Thanks for the last look over, I believe this might be all good now.

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @justin-richling! Just one final change request. After that is done if all testing passes on your end then feel free to merge it in. Thanks!

lib/adf_diag.py Outdated
Comment on lines 1128 to 1129
if glob.glob(os.path.join(ts_dir, f"*{hist_str}*.{constit}.*.nc")):
constit_files.append(glob.glob(os.path.join(ts_dir, f"*{hist_str}*.{constit}.*"))[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might have a check here for when hist_str = None:

Suggested change
if glob.glob(os.path.join(ts_dir, f"*{hist_str}*.{constit}.*.nc")):
constit_files.append(glob.glob(os.path.join(ts_dir, f"*{hist_str}*.{constit}.*"))[0])
if hist_str:
const_glob_str = f"*{hist_str}*.{constit}.*.nc"
else:
const_glob_str = f"*.{constit}.*.nc"
#end if
if glob.glob(os.path.join(ts_dir, const_glob_str)):
constit_files.append(glob.glob(os.path.join(ts_dir, const_glob_str ))[0])

@justin-richling justin-richling merged commit 911bea9 into NCAR:main Jun 24, 2024
7 checks passed
@justin-richling justin-richling deleted the aerosol-fix branch June 24, 2024 15:19
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.

2 participants