-
Notifications
You must be signed in to change notification settings - Fork 32
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
CDAT Migration: Refactor polar
set
#749
CDAT Migration: Refactor polar
set
#749
Conversation
Using this script:
I get:
|
@chengzhuzhang @tomvothecoder Are the testing directions at https://e3sm-project.github.io/e3sm_diags/_build/html/main/dev_guide/testing.html up to date? I tried running
|
I don't recall any changes to how tests are run on the |
0dfc01d
to
d1aec82
Compare
7833bda
to
8cde51d
Compare
I recently squashed the Can you make sure to checkout the latest version of his branch? I also recommend stashing and reapplying any changes you might have. |
340d3b3
to
56c7ffe
Compare
Note to self: see forsyth2#4 (#752) for an example. |
@tomvothecoder What am I supposed to rebase to remove that initial commit for #677 (d1aec82)? If I run |
Re: testing this branch: I basically took the tests from #752 and updated them to be more like the
and
|
Oh I just realized the directory information is wrong. Rerunning |
75b3ef3
to
f9b19de
Compare
Using the allocation command from #767. A lot of
|
3a85f49
to
b4cc7de
Compare
I tried changing regions to be |
Ran into this permissions issue while testing on Perlmutter: #772 |
@tomvothecoder Based on #749 (comment), I think I have actually removed all the CDAT dependencies from Re: these lines:
These refer to |
56c7ffe
to
59781c8
Compare
@tomvothecoder Yes, I followed those steps but when I clicked the "Execute Cell" play-button, it prompted "Type to choose a kernel source", with no kernels suggested. It had a dropdown suggestion of installing a Jupyter extension. I did that and tried again, clicking on However, that first cell now gives:
|
When you see Each notebook has a different environment in the instructions. It can probably be simplified to be a single environment, but I didn't write them with that in mind. |
6cdaaf0
to
dcbd32f
Compare
- Fix comments in `lat_lon_plot.py` and `zonal_mean_2d_plot.py`
polar
set
This function makes sure that the axes/axes being subsetted on is in | ||
ascending order. For example, if the latitude axis is in descending order, | ||
[90, 0, -90], no matches will be made on the subset if the region has a lat | ||
slice spec of (-90, -55) (e.g., 'polar_S'). This is because Xarray subsets | ||
in ascending order. Sorting the axis beforehand will avoid this issue. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chengzhuzhang Just an FYI about this fix with regional subsetting with Xarray using the region specs dictionary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few ref variables were not being subsetted correctly because the lat spec for polar_S
is (-90, -55)
but the lat coords were in descending order ([90, 0,... -90]
).
I completed refactoring with successful regression test. All variables are within the relative diff tolerance of 1e-5. I will be merging this PR once the CI/CD build passes. Polar viewers: |
0247f05
into
E3SM-Project:cdat-migration-fy24
Description
Refactor the polar set. Following the directions at https://github.com/E3SM-Project/e3sm_diags/wiki/CDAT-Migration-FY24-%E2%80%90-General-Guide#getting-started:
polar
set #659Summary of Changes:
polar_driver.py
andpolar_plot.py
driver/utils/regrid.py
subset_and_align_datasets()
to make copies of dataset objects before performing computations for cleaner code and ease of debugging_subset_on_region()
not subsetting specific regions correctly due to order of subsetting tuple. This function makes sure that the axes/axes being subsetted on is in ascending order. If it is not in ascending order, some slice specifications might have two negative values (e.g., (-90, -55) for 'polar_S') which can result in the subset incorrectly excluding of values that seem like they don't fit within the slice slice spec range when they actually are (circular coordinates).plot/utils.py
rect
arg to_add_colorbar()
for adjustment of therect
used for creating colorbar, along with_get_rect()
, which can vary based on the setleft_text_pos
andright_text_pos
args to_add_min_mean_max_test()
andadd_rmse_and_corr_text()
for adjustment of the x and y position of each respective text, which can vary based on the setcosp_histogram_py
_save_plot()
zonal_mean_2d_plot.py
auxiliary_tools/cdat_regression_testing/template_run_script.py
chown
command withchmod
.Checklist
If applicable: