One-step sfc variables time alignment #214
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbren12
suggested changes
Apr 3, 2020
workflows/one_step_jobs/runfile.py
Outdated
| return ( | ||
| sfc[list(SFC_VARIABLES)] | ||
| sfc[list(SFC_VARIABLES + GRID_VARIABLES)] |
There was a problem hiding this comment.
This is fine, but I would use the _safe_get_variables I defined below and see its docstring for why.
There was a problem hiding this comment.
I do realize I wrote the original list here, but it's worth a refactor now.
workflows/one_step_jobs/runfile.py
Outdated
| @@ -224,6 +240,8 @@ def post_process( | |||
| ) | |||
|
|
|||
| merged = xr.merge([sfc, ds]) | |||
| # assign step coordinate dtype, otherwise it's cast to object and zarr will choke | |||
| merged = merged.assign_coords({"step": merged["step"].values.astype("<U14")}) | |||
There was a problem hiding this comment.
We should somewhere keep track of useful conversions like this so we can reuse them.
There was a problem hiding this comment.
Like as a util function?
| ) | ||
|
|
||
| GRID_VARIABLES = ("lat", "lon", "latb", "lonb", "area") | ||
|
|
||
|
|
||
| def rename_sfc_dt_atmos(sfc: xr.Dataset) -> xr.Dataset: |
There was a problem hiding this comment.
This function does more than just renaming at this point. I would refactor the aligning logic to its own function, which is called from calling routine.
workflows/one_step_jobs/runfile.py
Outdated
| def _align_sfc_step_dim(da: xr.DataArray) -> xr.DataArray: | ||
|
|
||
| da_shift = da.shift(shifts={"time": 1}) | ||
| da_begin = da_shift.expand_dims({"step": ["begin"]}) |
There was a problem hiding this comment.
Following comments by @AnnaKwa, I took pains to remove all the hard-codes to the names like "after_dynamics" to into the "paths" dict at line 282, please use the names from that dict, which are the keys to monitor_paths in post_process.
brianhenn
commented
Apr 3, 2020
workflows/one_step_jobs/runfile.py
Outdated
| @@ -215,7 +231,7 @@ def post_process( | |||
| sfc = xr.open_mfdataset(sfc_pattern, concat_dim="tile", combine="nested").pipe( | |||
| rename_sfc_dt_atmos | |||
| ) | |||
| sfc = _safe_get_variables(sfc, SFC_VARIABLES) | |||
| sfc = _safe_get_variables(sfc, SFC_VARIABLES + GRID_VARIABLES) | |||
There was a problem hiding this comment.
Deleted this since it's getting called twice
brianhenn
changed the title
nbren12
approved these changes
Apr 6, 2020
nbren12
added a commit
that referenced
this pull request
Apr 13, 2020
* Feature/one step save baseline (#193) This adds several features to the one-step pipeline - big zarr. Everything is stored as one zarr file - saves physics outputs - some refactoring of the job submission. Sample output: https://gist.github.com/nbren12/84536018dafef01ba5eac0354869fb67 * save lat/lon grid variables from sfc_dt_atmos (#204) * save lat/lon grid variables from sfc_dt_atmos * Feature/use onestep zarr train data (#207) Use the big zarr from the one step workflow as input to the create training data pipeline * One-step sfc variables time alignment (#214) This makes the diagnostics variables appended to the big zarr have the appropriate step and forecast_time dimensions, just as the variables extracted by the wrapper do. * One step zarr fill values (#223) This accomplishes two things: 1) preventing true model 0s from being cast to NaNs in the one-step big zarr output, and 2) initializing big zarr arrays with NaNs via full so that if they are not filled in due to a failed timestep or other reason, it is more apparent than using empty which produces arbitrary output. * adjustments to be able to run workflows in dev branch (#218) Remove references to hard coded dims and data variables or imports from vcm.cubedsphere.constants, replace with arguments. Can provide coords and dims as args for mappable var * One steps start index (#231) Allows for starting the one-step jobs at the specified index in the timestep list to allow for testing/avoiding spinup timesteps * Dev fix/integration tests (#234) * change order of required args so output is last * fix arg for onestep input to be dir containing big zarr * update end to end integration test ymls * prognostic run adjustments * Improved fv3 logging (#225) This PR introduces several improvements to the logging capability of our prognostic run image - include upstream changes to disable output capturing in `fv3config.fv3run` - Add `capture_fv3gfs_func` function. When called this capture the raw fv3gfs outputs and re-emit it as DEBUG level logging statements that can more easily be filtered. - Refactor `runtime` to `external/runtime/runtime`. This was easy since it did not depend on any other module in fv3net. (except implicitly the code in `fv3net.regression` which is imported when loading the sklearn model with pickle). - updates fv3config to master * manually merge in the refactor from master while keeping new names from develop (#237) * lint * remove logging from testing * Dev fix/arg order (#238) * update history * fix positional args * fix function args * update history * linting Co-authored-by: Anna Kwa <[email protected]> Co-authored-by: brianhenn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes the diagnostics variables appended to the big zarr have the appropriate step and forecast_time dimensions, just as the variables extracted by the wrapper do.