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.

Sign up for GitHub

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

Jump to bottom

Bugfix/atrayano/#1252 defer #1253

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Bugfix/atrayano/#1252 defer #1253

wants to merge 3 commits into from

Conversation

atrayano
Copy link
Contributor

@atrayano atrayano commented Dec 27, 2021 •
edited by mathomp4
Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (affects only documentation or cleanup)

Checklist:

  • I have tested this change with a run of GEOSgcm (if non-trivial)
  • I have added one of the required labels (0 diff, 0 diff trivial, 0 diff structural, non 0-diff)
  • I have updated the CHANGELOG.md accordingly following the style of Keep a Changelog

@atrayano atrayano added 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. bugfix labels Dec 27, 2021
@atrayano atrayano requested a review from a team as a code owner December 27, 2021 17:30
@atrayano atrayano self-assigned this Dec 27, 2021
@mathomp4
Copy link
Member

mathomp4 commented Jan 3, 2022

Whoops. I missed this. Let me do a quick test as we'll get it in!

@mathomp4 mathomp4 added the 🚫 Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Jan 3, 2022
@mathomp4
Copy link
Member

mathomp4 commented Jan 3, 2022

@atrayano I put a blocker on this because this PR seems to have changed the metadata of some of the checkpoints:

  • aiau_import
  • moist_import
  • surf_import
  • turb_import

To wit, if you use nccmp -mfB you get:

Comparing NC4 aiau_import_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : AK : 2 <> 1 : VALUES : "Pa" : "1"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : BK : 2 <> 1 : VALUES : "Pa" : "1"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : DELP : 22 <> 18 : VALUES : "air_pressure_thickness" : "pressure_thickness"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : PHIS : 8 <> 7 : VALUES : "m2 sec-2" : "m+2 s-2"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : PS : 20 <> 16 : VALUES : "surface_air_pressure" : "surface_pressure"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : TS : 16 <> 24 : VALUES : "skin_temperature" : "surface_skin_temperature"
DIFFER : VARIABLE : TV : ATTRIBUTE : long_name : VALUES : "virtual_air_temperature" <> "air_virtual_temperature"
...
Comparing NC4 moist_import_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : EDMF_FRC : 25 <> 28 : VALUES : "Mass_Flux_Fractional_Area" : "EDMF_updraft_fractional_area"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : HL2 : 38 <> 35 : VALUES : "variance_of_liquid_water_static_energy" : "liquid_water_static_energy_variance"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : HL2 : 3 <> 2 : VALUES : "K+2" : "K2"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : HL3 : 42 <> 39 : VALUES : "third_moment_of_liquid_water_static_energy" : "third_moment_liquid_water_static_energy"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : HL3 : 3 <> 2 : VALUES : "K+3" : "K3"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : HLQT : 74 <> 50 : VALUES : "covariance_of_liquid_water_static_energy_and_total_water_specific_humidity" : "covariance_of_liquid_static_energy_and_total_water"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : KH : 18 <> 24 : VALUES : "scalar_diffusivity" : "total_scalar_diffusivity"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : KPBL : 30 <> 12 : VALUES : "planetary_boundary_layer_level" : "pbltop_level"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : KPBL_SC : 32 <> 24 : VALUES : "boundary_layer_level_for_UW_shlw" : "pbltop_level_for_shallow"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : RADSW : 40 <> 41 : VALUES : "air_temperature_tendency_due_to_longwave" : "air_temperature_tendency_due_to_shortwave"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : SNOMAS : 5 <> 6 : VALUES : "kg/m2" : "kg m-2"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : TAUX : 30 <> 23 : VALUES : "eastward_surface_stress_on_air" : "eastward_surface_stress"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : TAUY : 31 <> 24 : VALUES : "northward_surface_stress_on_air" : "northward_surface_stress"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : W2 : 29 <> 31 : VALUES : "variance_of_vertical_velocity" : "second_moment_vertical_velocity"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : W3 : 33 <> 30 : VALUES : "third_moment_of_vertical_velocity" : "third_moment_vertical_velocity"
DIFFER : VARIABLE : WHL : ATTRIBUTE : long_name : VALUES : "Liquid_water_static_energy_flux" <> "liquid_water_static_energy_flux"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : WHL : 7 <> 1 : VALUES : "K m s-1" : "1"
DIFFER : VARIABLE : WQT : ATTRIBUTE : long_name : VALUES : "Total_water_flux" <> "total_water_flux"
DIFFER : LENGTHS : ATTRIBUTE : units : VARIABLE : WQT : 13 <> 1 : VALUES : "kg kg-1 m s-1" : "1"
...
Comparing NC4 surf_import_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : DFNIRN : 47 <> 56 : VALUES : "normalized_surface_downwelling_nir_diffuse_flux" : "normalized_surface_downwelling_nearinfrared_diffuse_flux"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : DFUVRN : 47 <> 55 : VALUES : "normalized_surface_downwelling_uvr_diffuse_flux" : "normalized_surface_downwelling_ultraviolet_diffuse_flux"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : DRNIRN : 44 <> 53 : VALUES : "normalized_surface_downwelling_nir_beam_flux" : "normalized_surface_downwelling_nearinfrared_beam_flux"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : DRUVRN : 44 <> 52 : VALUES : "normalized_surface_downwelling_uvr_beam_flux" : "normalized_surface_downwelling_ultraviolet_beam_flux"
...
Comparing NC4 turb_import_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : VARIABLE : BSTAR : DOES NOT EXIST IN "stock-gcm-2022Jan03-1day-c24//scratch/turb_import_checkpoint"
DIFFER : VARIABLE : EVAP : DOES NOT EXIST IN "stock-gcm-2022Jan03-1day-c24//scratch/turb_import_checkpoint"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : RADLWC : 36 <> 56 : VALUES : "clearsky_air_temperature_tendency_lw" : "air_temperature_tendency_due_to_longwave_for_clear_skies"

Note: the data in these files is the same (well, sort of, see below), but the metadata has changed. Maybe that is what we want?

One oddity is with the turb_import one. This update to MAPL adds two new variables, BSTAR and EVAP! cdo sinfon confirms:

$ cdo sinfon stock-gcm-2022Jan03-1day-c24/scratch/turb_import_checkpoint
   File format : NetCDF4
    -1 : Institut Source   T Steptype Levels Num    Points Num Dtype : Parameter name
     1 : unknown  unknown  c instant      72   1      3456   1  F32  : OMEGA
     2 : unknown  unknown  c instant       1   2      3456   1  F32  : PHIS
     3 : unknown  unknown  c instant      72   1      3456   1  F32  : RADLW
     4 : unknown  unknown  c instant      72   1      3456   1  F32  : RADLWC
     5 : unknown  unknown  c instant       1   2      3456   1  F32  : SH
     6 : unknown  unknown  c instant      72   1      3456   1  F32  : WTHV2
...
$ cdo sinfon atanasmapl-gcm-2022Jan03-1day-c24/scratch/turb_import_checkpoint
   File format : NetCDF4
    -1 : Institut Source   T Steptype Levels Num    Points Num Dtype : Parameter name
     1 : unknown  unknown  c instant       1   1      3456   1  F32  : BSTAR
     2 : unknown  unknown  c instant       1   1      3456   1  F32  : EVAP
     3 : unknown  unknown  c instant      72   2      3456   1  F32  : OMEGA
     4 : unknown  unknown  c instant       1   1      3456   1  F32  : PHIS
     5 : unknown  unknown  c instant      72   2      3456   1  F32  : RADLW
     6 : unknown  unknown  c instant      72   2      3456   1  F32  : RADLWC
     7 : unknown  unknown  c instant       1   1      3456   1  F32  : SH
     8 : unknown  unknown  c instant      72   2      3456   1  F32  : WTHV2

Again, perhaps this is actually fixing a bug we've long had?

tclune
tclune approved these changes Jan 3, 2022
View reviewed changes
Copy link
Collaborator

@tclune tclune left a comment

Choose a reason for hiding this comment

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

Seems clean enough.

@stale
Copy link

stale bot commented Mar 4, 2022

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days, it will be closed. You can add the "long term" tag to prevent the Stale bot from closing this issue.

@stale stale bot added the ❄️ Stale This issue has been marked stale label Mar 4, 2022
@atrayano atrayano added the ⌛ Long Term Long term issues label Mar 4, 2022
@stale stale bot removed the ❄️ Stale This issue has been marked stale label Mar 4, 2022
@tclune tclune removed the bugfix label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@818Nawaf 818Nawaf 818Nawaf approved these changes

@tclune tclune tclune approved these changes

@bena-nasa bena-nasa Awaiting requested review from bena-nasa

Assignees

@atrayano atrayano

Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch. ⌛ Long Term Long term issues 🚫 Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@atrayano @mathomp4 @tclune @818Nawaf