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

Bnb/dev #173

Merged
merged 51 commits into from
Nov 9, 2023
Merged

Bnb/dev #173

merged 51 commits into from
Nov 9, 2023

Conversation

bnb32
Copy link
Collaborator

@bnb32 bnb32 commented Nov 3, 2023

Bc code for using vortex vs era means to compute factors. Some small tweaks for dual handler use with different sets of features for lr and hr handlers.

…d to make sure different sets of features are indexed carefully. cape added to era_downloader. added vortex mean based bias correction code.
…same features originally but this isnt required. means/stds have to be indexed carefully.
…same features originally but this isnt required. means/stds have to be indexed carefully.
…spatial fwp with netcdf output and then temporal fwp on that output)
@bnb32 bnb32 force-pushed the bnb/dev branch 2 times, most recently from 7cc968f to 91f2b04 Compare November 4, 2023 22:39
"""

def __init__(self, path_pattern, in_heights, out_heights, overwrite=False):
"""Parameters
Copy link
Member

Choose a reason for hiding this comment

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

Minor but i wonder if you need a line break to get the parameters to show up in the docs page

@@ -0,0 +1,949 @@
"""Classes to compute means from vortex and era data and compute bias
correction factors.
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to include any details on where / how to download the vortex means? No code just a quick tip.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah good idea.

"""Parameters
----------
path_pattern : str
Pattern for input tif files. Needs to include {month} and {height}
Copy link
Member

Choose a reason for hiding this comment

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

surprised no assert statement in the init for these format keys

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Me too. What was past Brandon thinking?

):
"""Read vortex tif files, convert these to monthly netcdf files for all
input heights, interpolate this data to requested output heights, mask
fill values, and write all data to h5 file.
Copy link
Member

Choose a reason for hiding this comment

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

Seems kind of weird that you need the netcdf intermediate step but im assuming you thought of this and need it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this was just an easy way to combine all the heights (xr.open_mfdataset()) in a single file which could then go through the normal interpolation routine.

logger.info(f"Finished writing means for {out_file}.")

@classmethod
def run(cls, era_pattern, years, features, out_pattern):
Copy link
Member

Choose a reason for hiding this comment

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

The ERA netcdf files should be able to be handled by the standard DataHandlerNC classes and the BiasCalc class. Why didnt you just convert vortex data to a standard format and then use the BiasCalc class from there? I think the mods to allow the BiasCalc module to use monthly data would be really simple

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Classic example of working harder and not smarter.

max_workers=self.hr_dh.norm_workers)

@property
def output_features(self):
"""Get list of output features. e.g. those that are returned by a
GAN"""
return self.hr_dh.output_features
return self.lr_dh.output_features
Copy link
Member

Choose a reason for hiding this comment

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

This is surprising, maybe i'm missing something, why wouldnt the high-res data handler have the correct output feature list? Just because the low-res handler has a list of "train only" features that makes this correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

High-res should have the correct output feature list but it just felt cleaner to use lr_dh for both self.features and self.output_features.

@@ -219,7 +222,7 @@ def _run_pair_checks(self, hr_handler, lr_handler):
assert hr_handler.val_split == 0 and lr_handler.val_split == 0, msg
msg = ('Handlers have incompatible number of features. '
f'({hr_handler.features} vs {lr_handler.features})')
assert hr_handler.features == lr_handler.features, msg
assert hr_handler.features == self.output_features, msg
Copy link
Member

Choose a reason for hiding this comment

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

Oh maybe this confirms my previous question?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep!

…nform to format expected by classes in bias_calc.py. Added scalar only class to bias_calc.py
@grantbuster
Copy link
Member

@bnb32 my only questions is whether we want to keep the BiasCorrectionFromMeans or are you integrating with the bias_calc.py module? Also, i guess scalar bias correction is linear but i'd still recommend a shorter name than MonthlyLinearCorrectionScalarOnly, maybe MonthlyScalarCorrection

@bnb32 bnb32 merged commit b9e3fd6 into main Nov 9, 2023
8 checks passed
@bnb32 bnb32 deleted the bnb/dev branch November 9, 2023 21:39
github-actions bot pushed a commit that referenced this pull request Nov 9, 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.

2 participants