-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add lat_lon_land from e3sm_diags for land diagnostics #548
Conversation
The results include 3 types for e3sm_diags runs:
Link to the viewers: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.zhang40/tests/E3SMv3_dev/land_diags_try8/20231209.v3.LR.piControl-spinup.chrysalis/ Update: The configuration file that works so far:
|
@chengzhuzhang Thanks for figuring out the necessary changes to add |
I'm trying to debug the complete_run failures. I'm completely baffled that Working as of #424 (just merged):
Failing in my complete_run test of this PR:
These are identical aside from the output/www path. Yet, testing this PR gives me:
For reference, the following is the relevant sections from the working cfg in #548 (comment)
I tried running with all those variables earlier, but ran into a number of regridding failures. |
@forsyth2 could you clarify if the test complete run cfg is updated by you? If yes, what is the change? It would be easier for trouble shooting if you can commit the code change. |
The regridding failures could be from the misuse of mapping file. In my example, I used map_r05_to_cmip6_180x360_aave.20231110.nc for tri-grid v3 output, but for the v2 output you have been testing, ne30gp2_to cmip6 is needed. |
Yes, I couldn't use your cfg exactly because you used different input (notably v3 rather than v2).
See updates to tests/integration/template_complete_run.cfg in 61f99c3 and 998c87d. I'm going to try rebasing the code on to the latest
Yes, I noticed the mapping file change but found that indeed v2 needs the original mapping file. |
No, still fails:
There's nothing in this PR that would cause that task to fail. It just worked when testing #424. I don't understand how it could possibly be a flaky test (https://docs.gitlab.com/ee/development/testing_guide/flaky_tests.html)... |
I will try if i can reproduce this afternoon |
@forsyth2 I checked the test for PR 424. /lcrc/group/e3sm/ac.forsyth2/zppy_test_complete_run_output/test-424/v2.LR.historical_0201/post/lnd/180x360_aave/ts/monthly/2yr/ |
@chengzhuzhang Thanks for checking. I did a thorough review of what's going on in #549. For the purposes of testing this, I will re-run jobs until I get dependencies succeeding, and then make sure the integration tests pass. Testing will have this added complication until #549 can be fully resolved... |
Test status so far (running on a rebased commit I haven't pushed yet): Ran
=>
|
Re-running
Further rerunsI tried re-running various times, a few times with Ultimately though, running ILAMB is able to run once that dependency is taken care of. Still running into an E3SM Diags error though:
|
@forsyth2 what is the e3sm_diags error? the reason |
Ah, so should I be using the 180x360 grid, as before? Follow-up: should |
Yes, for now, we should continue to use the 180x360grid when testing v2. But should transition to use v3 datasets, or with a new configure file. |
|
Now that #549 has resolved, I'm running the tests on this PR so that I can update the expected images to include |
Interestingly still seeing the |
Never mind, I still had Also, I didn't see |
Actually the error does seem to still happen:
|
f7ed028
to
77fe3fb
Compare
Yes, it does look like the
|
Searching for the error @tomvothecoder In this case, it might be worth to try by using |
Thanks for finding this helpful thread! Jason might have been right that the filesystem on the server does not behave well with Xarray's parallel access at random times. I'll make a new release of e3sm_to_cmip with |
e3sm_to_cmip v1.11.2rc2 will be available on Feedstock PR: conda-forge/e3sm_to_cmip-feedstock#35 |
@forsyth2 did you have a chance to test the new e3sm_to_cmip version? |
@chengzhuzhang I began testing yesterday and will have a full update later today. So far, so good though. |
8e5b1af
to
bd70de6
Compare
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 Tests now passing. @tomvothecoder Looks like the latest e3sm_to_cmip
fully resolves the concurrency error.
Resolves #518.