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

Fix bug in usage of FRSNO met-field #2254

Merged
merged 8 commits into from
Apr 26, 2024
Merged

Fix bug in usage of FRSNO met-field #2254

merged 8 commits into from
Apr 26, 2024

Conversation

lizziel
Copy link
Contributor

@lizziel lizziel commented Apr 18, 2024

Name and Institution (Required)

Name: Lizzie Lundgren
Institution: Harvard University

Describe the update

This PR fixes a bug where State_Met%FRSNO is inconsistently used. In some places it is used as fraction of grid box with snow covered land, while in other places it is used as fraction of land with snow cover. The GMAO meteorology field input to the model is defined as fraction of land with snow cover. To fix the issue we now define State_Met%FRSNO (renamed to State_Met%FRSNOW) as fraction grid box with snow cover on land, and compute it upon file read by multiplying the FRSNO import with fraction land import (FRLAND). Usage of the met-field throughout the model is now updated for consistency.

Also in this update is renaming State_Met%LANDIC to State_Met%LANDICE for clarity, and adding diagnostics for State_Met%IsWater, State_Met%IsLand, State_Met%IsIce, and State_Met%IsSnow for sanity checking. The surface type variables are type logical but the diagnostics are stored in State_Diag as REAL4.

Expected changes

This bug fix has the most impact for GCHP runs that use raw GMAO meteorology files. The raw files have missing values for all non-land grid boxes and FRSNO was set to 1e15 in those locations, resulting in incorrect logical masks for water, snow, and ice. The new multiplication by FRLAND removes that issue.

Mercury simulations are impacted because State_Met%FRSNO was used as if it were fraction grid box rather than fraction land.

Very small differences may be seen in GC-Classic full chemistry runs and GCHP runs that use processed meteorology files due to small changes in time-varying State_Met%FRSNOW when computing surface type masks (water, snow, ice, land). These changes would only be where snow is present.

Reference(s)

None

Related Github Issue(s)

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
This update makes the definitions more descriptive to avoid confusion
about whether fraction is per grid box or per sub-type, e.g. land.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel added this to the 14.4.0 milestone Apr 18, 2024
@lizziel lizziel added category: Bug Something isn't working topic: Input Data Related to input data Attn: Hg-POPs WG labels Apr 18, 2024
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel force-pushed the bugfix/frsno branch 2 times, most recently from 2a11c74 to 4d2a42c Compare April 19, 2024 18:53
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel marked this pull request as ready for review April 19, 2024 20:58
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
GeosCore/calc_met_mod.F90 Show resolved Hide resolved
GeosCore/flexgrid_read_mod.F90 Show resolved Hide resolved
Headers/state_met_mod.F90 Outdated Show resolved Hide resolved
Interfaces/GCHP/Includes_Before_Run.H Show resolved Hide resolved
GeosCore/calc_met_mod.F90 Show resolved Hide resolved
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
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.

I also approve! Thanks @lizziel.

@yantosca yantosca self-assigned this Apr 24, 2024
@yantosca
Copy link
Contributor

Integration tests are running

@yantosca yantosca added the topic: Configuration Files Related to GEOS-Chem configuration files label Apr 25, 2024
@yantosca
Copy link
Contributor

All GEOS-Chem Classic execution tests passed:

==============================================================================
GEOS-Chem Classic: Execution Test Results

GCClassic #2fa1736 Update RTD docs for PR #2266 (Remove obsolete carbon sim options)
GEOS-Chem #203a480b6 Merge PR #2254 (Fix bug in usage of FRSNO met field)
HEMCO     #fddcae5 Update version to 3.8.1 for release

Using 24 OpenMP threads
Number of execution tests: 29

Submitted as SLURM job: 29651290
==============================================================================

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

All GCHP integration tests passed:

==============================================================================
GCHP: Execution Test Results

GCHP      #e86ba15 GEOS-Chem submod update: Merge PR #2266 (Remove unused carbon sim options)
GEOS-Chem #203a480b6 Merge PR #2254 (Fix bug in usage of FRSNO met field)
HEMCO     #fddcae5 Update version to 3.8.1 for release

Number of execution tests: 11

Submitted as SLURM job: 29652416
==============================================================================

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

@yantosca yantosca merged commit 203a480 into dev/14.4.0 Apr 26, 2024
@yantosca yantosca deleted the bugfix/frsno branch April 26, 2024 13:37
@msulprizio msulprizio added the topic: Hg or POPs simulations Related to the GEOS-Chem Mercury Simulation label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Bug Something isn't working topic: Configuration Files Related to GEOS-Chem configuration files topic: Hg or POPs simulations Related to the GEOS-Chem Mercury Simulation topic: Input Data Related to input data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants