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

Changes to MOPITT pairing #316

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

Conversation

mbruckner-work
Copy link
Collaborator

Adds utility for sampling model data to satellite local overpass time.

Modifies MOPITT L3 pairing to calculate model averages using only model data at the satellite overpass time.
Also ensures that model is paired to correct obs time. Previously assumed that model and obs datasets were on same time at that point, but this isn't true if there are gaps in the obs dataset.

New argument for the yaml file if model output hasn't been pre-processed to monthly mean: mod_to_overpass.

Note: Insert/delete flagging includes some commits from other recent pull requests (#304, #312 ).

@rschwant rschwant requested a review from rrbuchholz February 10, 2025 18:32
if obs.obs_type.lower() in list(self.pairing_kwargs.keys()):
pairing_kws = self.pairing_kwargs[obs.obs_type.lower()]
else:
pairing_kws = {'apply_ak': True, 'mod_to_overpass': False}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add an error message as to why MM might break if no AK is available.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the message could be made a bit more helpful by telling you why it's being set to true (defaulting since not set) and how you (the user) could fix it.

Copy link
Collaborator

@rrbuchholz rrbuchholz 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 to me. Ready to merge once those other 2 Level3 options are added.

@mbruckner-work mbruckner-work marked this pull request as ready for review February 13, 2025 18:15
@rrbuchholz
Copy link
Collaborator

Looks like Maggie made those few updates and it is ready to merge -- @rschwant or @zmoon do you agree?

rrbuchholz

This comment was marked as duplicate.

Copy link
Collaborator

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Added a few comments.

@@ -53,7 +103,7 @@ def check_timestep(model_data,obs_data):
print('Timestep check and model resample failed')
raise

def mopitt_l3_pairing(model_data,obs_data,co_ppbv_varname):
def mopitt_l3_pairing(model_data,obs_data,co_ppbv_varname,global_m=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not global_model? Only 4 more chars.

ufsaqm:
files: '/scratch1/BMC/rcm2/rhs/monet_example/AEROMMA/UFS-AQM/cmaq54_OriRave1/aqm.202306*/12/aqm.t12z.dyn.*.nc'
mod_type: 'rrfs'
is_global: False
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the driver code I'm seeing isglobal, but I like this (is_global) better. May want to fix the driver.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also it doesn't seem to have been added to the YAML doc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed and added a description to YAML doc.

Comment on lines 55 to 65
# Determine local time offset
local_utc_offset = np.zeros([ny,nx],dtype='timedelta64[ns]')
# pandas timedelta calculation doesn't work on ndarrays
for xi in np.arange(nx):
local_utc_offset[:,xi] = pd.to_timedelta((modobj['longitude'].isel(x=xi)/15).astype(np.int64),unit='h')

# initialize local time as variable
modobj['localtime'] = (['time','y','x'],np.zeros([nmt,ny,nx],dtype='datetime64[ns]'))
# fill
for ti in np.arange(nmt):
modobj['localtime'][ti] = modobj['time'][ti].data + local_utc_offset
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may want to round lon/15 instead of truncating by converting to int. You could try the approach here to get around the pd.to_timedelta limitation:

local_utc_offset = (modobj['longitude'] / 15).round().astype("timedelta64[h]")

Then you should be able to do

modobj['localtime'] = modobj['time'] + local_utc_offset

if obs.obs_type.lower() in list(self.pairing_kwargs.keys()):
pairing_kws = self.pairing_kwargs[obs.obs_type.lower()]
else:
pairing_kws = {'apply_ak': True, 'mod_to_overpass': False}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the message could be made a bit more helpful by telling you why it's being set to true (defaulting since not set) and how you (the user) could fix it.

Comment on lines 1342 to 1347
# grab kwargs for pairing. Use default if not specified
if obs.obs_type.lower() in list(self.pairing_kwargs.keys()):
pairing_kws = self.pairing_kwargs[obs.obs_type.lower()]
else:
print('WARNING: The satellite pairing option apply_ak is being set to True. Pairing will fail if there is no AK available.')
pairing_kws = {'apply_ak': True, 'mod_to_overpass': False}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently you'd have an issue later if pairing_kwargs for sat were set in the control but didn't contain apply_ak, e.g. just mod_to_overpass or another unrelated setting. An alternative approach would be to set pairing_kws to your desired defaults first and then update it based on pairing_kwargs from the control. That way you know everything is set. There would still be an opportunity to let the user know that they may need to set apply_ak.

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