Skip to content

Conversation

breichl
Copy link

@breichl breichl commented Oct 22, 2021

  • Mixed Layer depth diagnostic code added for analyzing model output
  • Takes MLD_003, MLD_EN1, MLD_EN2, OR MLD_EN3 as inputs.
  • Creates map of input (either min or max of monthly means)
  • Compares to similar field from obs (usually Argo based).

Potential Issues:

  • "RuntimeWarning: All-NaN slice encountered" warning message printing out (presumably from the .min()/.max() operations)
  • Still using griddata

Command to test:
om4labs mld --grid global --method max --mldvar MLD_EN1 -s /archive/bgr/FMS2019.01.03_mom6_20201020/OM4p25_JRA55do1.5_ePBLRL19_BBL01_VS_JHLtol/gfdl.ncrc4-intel18-prod/pp/ocean_monthly/ocean_monthly.static.nc /archive/bgr/FMS2019.01.03_mom6_20201020/OM4p25_JRA55do1.5_ePBLRL19_BBL01_VS_JHLtol/gfdl.ncrc4-intel18-prod/pp/ocean_monthly/ts/monthly/5yr/ocean_monthly.199801-200212.MLD_EN1.nc /archive/bgr/FMS2019.01.03_mom6_20201020/OM4p25_JRA55do1.5_ePBLRL19_BBL01_VS_JHLtol/gfdl.ncrc4-intel18-prod/pp/ocean_monthly/ts/monthly/5yr/ocean_monthly.200301-200712.MLD_EN1.nc /archive/bgr/FMS2019.01.03_mom6_20201020/OM4p25_JRA55do1.5_ePBLRL19_BBL01_VS_JHLtol/gfdl.ncrc4-intel18-prod/pp/ocean_monthly/ts/monthly/5yr/ocean_monthly.200801-201212.MLD_EN1.nc

- Mixed Layer depth diagnostic code added for analyzing model output
- Takes MLD_003, MLD_EN1, MLD_EN2, OR MLD_EN3 as inputs.
- Creates map of input (either min or max of monthly means)
- Compares to similar field from obs (usually Argo based).
@jkrasting jkrasting self-requested a review October 25, 2021 12:51
Copy link
Collaborator

@jkrasting jkrasting left a comment

Choose a reason for hiding this comment

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

@breichl - Looks really good. Mostly a few minor nits.

Major comments:

  • Please apply the black code formatter to your .py files
  • Consider whether or not using xESMF for regridding would work better for this diagnostic

Comment on lines +188 to +189
obs_lat = np.copy(ds_obs["Lat"])
obs_lon = np.copy(ds_obs["Lon"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copying in general increases the memory footprint. You can access the underlying NumPy arrays by using the .values method, e.g.

obs_lat = ds_obs["Lat"].values

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in #111

# Gridding obs data to common grid. While obs data is already on the common grid,
# it may need shuffled if crossing periodic longitude boundary. This is an easy
# way to ensure that happens correctly.
obs = griddata((obs_lat.flatten(),obs_lon.flatten()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, there is a lot of code here to implement regridding functionality. I wonder if using xESMF would be more streamlined?

Copy link
Owner

Choose a reason for hiding this comment

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

I agree xesmf should be used instead of griddata, there is a function in om4common to regrid data

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in #111

# Gridding obs data to common grid. While obs data is already on the common grid,
# it may need shuffled if crossing periodic longitude boundary. This is an easy
# way to ensure that happens correctly.
obs = griddata((obs_lat.flatten(),obs_lon.flatten()),
Copy link
Owner

Choose a reason for hiding this comment

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

I agree xesmf should be used instead of griddata, there is a function in om4common to regrid data

@breichl
Copy link
Author

breichl commented Jul 25, 2023

This PR is replaced by #111

@breichl breichl closed this Jul 25, 2023
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.

3 participants