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

Bug fix: allow uppercase log filename in download_data.py #2508

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

ihough
Copy link

@ihough ihough commented Oct 10, 2024

Name and Institution (Required)

Name: Ian Hough
Institution: Université Grenoble Alpes

Describe the update

Fixes a bug where download_data.py fails if the dryrun log filename contains uppercase characters e.g. 'Log.dryrun'. The solution is to not coerce the dryrun log filename to lowercase.

Expected changes

No changes to model output

Reference(s)

None

Related Github Issue

#70

msulprizio and others added 3 commits September 11, 2024 14:03
… CO2

- Add missing ship logical to HEMCO_Config.rc.carbon, otherwise ship
  emissions for CO2 are not included
- Update HEMCO_Config.rc.CO2 to:
  1. Include DiagnFile to properly save out emissions diagnostics
  2. Use same Aviation_SurfCorr_SclFac.1x1.nc file as carbon simulation

Signed-off-by: Melissa Sulprizio <[email protected]>
Fix bug where download_data.py failed if the dryrun log filename contained uppercase characters
e.g. 'Log.dryrun'. The solution is to not coerce the dryrun log filename to lowercase.
@yantosca yantosca self-assigned this Oct 10, 2024
@yantosca yantosca added topic: Dry-Run Simulation Related to GEOS-Chem dry-run category: Bug Fix Fixes a previously-reported bug labels Oct 10, 2024
@yantosca yantosca self-requested a review October 10, 2024 14:03
@yantosca yantosca modified the milestones: 14.5.0, 14.5.1 Oct 10, 2024
Copy link
Contributor

@yantosca yantosca 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 this @ihough. It looks good to merge. But would you also be able to add a note in the CHANGELOG.md under the Unreleased / Changed section? We require the changelog update for all PRs. Thanks!

msulprizio and others added 2 commits October 11, 2024 12:42
…sed on species defined

In carbon_gases_mod.F90 all chemistry input fields needed for CO2, CH4, and CO
chemistry were obtained regardless of the species included in the carbon simulation.
Therefore when running the carbon simulation with only one species (e.g. CO2)
fields were being obtained that weren't necessary. These fields have been
blocked by logical statements checking if the species that needs those fields is
advected.

Also in this module, the chemistry routine has been updated to only call
KPP when CH4 and/or CO are defined as advected species. KPP is not needed
for CO2-only simulations. Instead CO2-only simulations utilize the CO2_COPROD
field obtained from HEMCO. This field was originally obtained and applied
to the species concentration array in subroutine Emiss_Carbon_Gases. The
routine name was misleading and has been renamed to CO2_Production. The call
to this routine has also been moved from emissions_mod.F90 to Chem_Carbon_Gases
in carbon_gases_mod.F90.

Notes: When we retire the CO2 simulation, we should be able to also remove
the CO2 menu from geoschem_config.yml since it is not needed when using
CO2 in the carbon simulation.

Signed-off-by: Melissa Sulprizio <[email protected]>
@ihough
Copy link
Author

ihough commented Oct 14, 2024

@yantosca updated the changelog.

The call to routine CO2_Production (formerly Emiss_Carbon_Gases) has been
moved back to emissions_mod.F90 to reproduce the CO2 simulation.

We may want to consider moving the call to this routine back to chemistry
(i.e. in Chem_Carbon_Gases) since the routine applies the production of
CO2 from CO and therefore represents a chemical source, not emissions.
I will follow up with the Carbon Gases Working Group for guidance.

Signed-off-by: Melissa Sulprizio <[email protected]>
…ased on species defines

Logical switches have been added to HEMCO_Config.rc.carbon to only read
the data necessary based on the carbon species used. For example, GLOBAL_OH
is not needed for CO2-only simulations and will therefore not be read anymore.
This should speed up single-species carbon simulations.

Signed-off-by: Melissa Sulprizio <[email protected]>
…U and on first timestep

Also fixed a typo in CHANGELOG.md to use past tense.

Signed-off-by: Melissa Sulprizio <[email protected]>
…2-only simulations

The carbon simulation with CO2 only was failing integration tests due to a
failed ExtData import for CEDS_CO2_SHIP. The sed statements in singleCarbonSpecies.sh
removed any entries with "CEDS_CO" if the CO species was not included in
the simulation, also inadvertently removing the CEDS_CO2_SHIP entry. This
has now been fixed by adding an underscore after CEDS_CO2. Other lines in
that sed statement have also been updated to include an underscore or trailing
white space to avoid this issue in the future.

Signed-off-by: Melissa Sulprizio <[email protected]>
Several entries for CO2 emissions and chemistry data were incorrect in
ExtData.rc.carbon. Primarily the climatology option and refresh string
were inconsistent with the entries in HEMCO_Config.rc.

Signed-off-by: Melissa Sulprizio <[email protected]>

# Conflicts:
#	CHANGELOG.md
…ons for consistency with carbon simulations

The CO2 and tagCO run directories were created using old restart files whose origin
is not clear. To simplify validating the carbon simulation, these simulations
now use the carbon simulation restart file which has been spun up for 5 years
with a recent model version (14.1.0). The default start date for these simulations
has changed to 20190101 as well.

Signed-off-by: Melissa Sulprizio <[email protected]>
@ihough ihough requested a review from yantosca November 7, 2024 14:05
ihough and others added 3 commits November 7, 2024 15:48
This merge brings in the updates from the 14.5.0 release back
into the dev/no-diff-to-benchmark branch.

See the CHANGELOG.md for updates.

Signed-off-by: Bob Yantosca <[email protected]>
This merge brings PR geoschem#2560 (Fix trace metals simulation name in
config file comments, by @ihough) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR fixes incorrect comments in configuration files.
Also, merge conflicts in CHANGELOG.md have been resolved.

Signed-off-by: Bob Yantosca <[email protected]
@yantosca
Copy link
Contributor

yantosca commented Nov 19, 2024

@ihough: Is it OK if I push back to this branch on your fork? That way the updates will show up on the PR. This will also bring in updates from 14.5.0.

This merge brings PR # (Bug fix: allow uppercase log filename
in download_data.py, by @ihough) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR adds a fix so that dry-run log file names that are
fed to the download_data.py script can have mixed case.
Merge conflicts in CHANGELOG.md have also been resolved.

Signed-off-by: Bob Yantosca <[email protected]>
@ihough
Copy link
Author

ihough commented Nov 19, 2024

@yantosca sure push away!

Copy link
Contributor

Thanks @ihough

@yantosca yantosca added the no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations label Nov 20, 2024
@yantosca
Copy link
Contributor

NOTE: I pushed updates before changing the base branch. This PR will close when merged into main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Bug Fix Fixes a previously-reported bug no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations topic: Dry-Run Simulation Related to GEOS-Chem dry-run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants