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

Code fails on assumption of 1 clim file per variable name not accounting for components (i.e precc, precl vs prect) or comparable variables (i.e ts vs sst) #228

Closed
juliecaron opened this issue Feb 2, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@juliecaron
Copy link
Collaborator

ADF run type

Model vs. Obs

What happened?

There are a couple of layers to this. But the source of the error is in regrid_and_vert_interp.py, in this section:
if len(mclim_fils) > 1:
#Combine all cam files together into a single data set:
mclim_ds = xr.open_mfdataset(mclim_fils, combine='by_coords')
else:
#Open single file as new xsarray dataset:
mclim_ds = xr.open_dataset(mclim_fils[0])
#End if

First, if one wants to use the yaml file for the default AMWG behavior, the code currently assumes that "PRECT" is there and does not check for PRECL and PRECC. but i believe the rest of the code can handle PRECL and PRECC (presumably the timeseries file creation combines the terms into PRECT, but i don't remember for sure), but in this case I am using timeseries files already generated offline, not via ADF.

Second, if one variable is not there (i.e. SST), it is also not trying to find TS and mask it. It looks for an SST clim file and fails.

So currently, in these instances, the code errors out because the len(mclim_fils) is actually zero. but the code snippet above assumes that it will only ever be mor than 1 or 1 (because the else has the action of subscripting assuming length 1). It'd be great to check if it's zero, and then give a clear error message "No clim file for variable X found. " . Additionally, it'd be great if it would check for comparable variables and combine (in case of prect) and/or use an alternate like TS for SST. and let the user know with "Using Y instead" or "Can't find alternate variable. Skipping X."

ADF Hash you are using

bcf3630

What machine were you running the ADF on?

CISL machine

What python environment were you using?

NPL (CISL machines only)

Extra info

No response

@juliecaron juliecaron added the bug Something isn't working label Feb 2, 2023
@juliecaron juliecaron changed the title Code fails on assumption of clim file for all variables in the list Code fails on assumption of 1 file per variable name not accounting for components or comparable variables (i.e precc, precl vs prect) Feb 2, 2023
@juliecaron juliecaron changed the title Code fails on assumption of 1 file per variable name not accounting for components or comparable variables (i.e precc, precl vs prect) Code fails on assumption of 1 clim file per variable name not accounting for components or comparable variables (i.e precc, precl vs prect) Feb 2, 2023
@juliecaron juliecaron changed the title Code fails on assumption of 1 clim file per variable name not accounting for components or comparable variables (i.e precc, precl vs prect) Code fails on assumption of 1 clim file per variable name not accounting for components (i.e precc, precl vs prect) comparable variables (i.e ts vs sst) Feb 2, 2023
@juliecaron juliecaron changed the title Code fails on assumption of 1 clim file per variable name not accounting for components (i.e precc, precl vs prect) comparable variables (i.e ts vs sst) Code fails on assumption of 1 clim file per variable name not accounting for components (i.e precc, precl vs prect) or comparable variables (i.e ts vs sst) Feb 2, 2023
justin-richling added a commit to justin-richling/ADF that referenced this issue Mar 20, 2023
Also fix small bugs:
* Move RESTOM to top of AMWG tables
* Missing check for len of climo files in regrid_and_vert_interp (fixes part of Issue NCAR#228)
* Special plots didn't work with multi-case diags initially
justin-richling added a commit to justin-richling/ADF that referenced this issue Mar 21, 2023
Also fix small bugs:
* Move RESTOM to top of AMWG tables
* Missing check for len of climo files in regrid_and_vert_interp (fixes part of Issue NCAR#228)
* Special plots didn't work with multi-case diags initially
@justin-richling
Copy link
Collaborator

Close via PR #260

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants