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

HRRR model levels #570

Merged
merged 17 commits into from
Jul 28, 2023
Merged

HRRR model levels #570

merged 17 commits into from
Jul 28, 2023

Conversation

bbuzz31
Copy link
Collaborator

@bbuzz31 bbuzz31 commented Jul 24, 2023

Use the native model levels rather than the pressure levels in order to cover a more complete height of the troposphere.

Closes #568

@dbekaert
Copy link
Owner

dbekaert commented Jul 25, 2023

Thanks Brett, this looks already much better based on your figure. One of the test seems to have an issue with the definition of the layers.

CHANGELOG.md Outdated Show resolved Hide resolved
@bbuzz31 bbuzz31 marked this pull request as ready for review July 25, 2023 18:02
@bbuzz31 bbuzz31 requested a review from dbekaert July 25, 2023 18:02
tools/RAiDER/models/ecmwf.py Show resolved Hide resolved
tools/RAiDER/models/hrrr.py Outdated Show resolved Hide resolved
tools/RAiDER/models/hrrr.py Outdated Show resolved Hide resolved
tools/RAiDER/models/hrrr.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jlmaurer jlmaurer left a comment

Choose a reason for hiding this comment

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

LGTM. I updated the NotImplementedError message on pressure levels to help me remember in the future why we aren't using them.

@@ -321,39 +308,24 @@ def _load_pressure_level(self, filename, *args, **kwargs):
self._t = t
self._q = q

geo_hgt = z / self._g0
geo_hgt = (z / self._g0).transpose(1, 2, 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jlmaurer here we transpose the geo_hgt, which then propagates to the z and p

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if you don't specify height levels, then all the model layers will be used. the top of the model layer (ht) is by definition = high_ht at the top. So nothing is calculated and then the stacking crashes. It's a very tiny contribution to the integral.

@bbuzz31
Copy link
Collaborator Author

bbuzz31 commented Jul 27, 2023

@cmarshak any idea why the GUNW test could be failing?

It writes the first delay successfully but then seems to kill itself during the next datetime:
The end of stdout is as follows:


Starting to run the weather model calculation
Date: 20200124
Beginning weather model pre-processing
Weather model HRRR is available from 2016-07-15 to Present
Weather model HRRR is available from 2016-07-15 to Present

It should continue with, e.g., ✅ Found ┊ model=hrrr ┊ product=nat ┊ 2020-Jan-30 13:00 UTC F00 ┊ GRIB2 @ aws ┊ IDX @

But instead just stops and returns:

subprocess.CalledProcessError: Command '['raider.py', '++process', 'calcDelaysGUNW', '-f', '/root/project/test/GUNW/S1-GUNW-D-R-071-tops-20200130_20200124-135156-34956N_32979N-PP-913f-v2_0_4.nc', '-m', 'HRRR', '-o', '/root/project/test/GUNW']' died with <Signals.SIGKILL: 9>

Maybe a memory issue on circleci? I can't reproduce it locally, and a main change here was to use the product=nat which is bigger

@cmarshak
Copy link
Collaborator

cmarshak commented Jul 27, 2023

I would run the test on a linux server via pytest test/test_GUNW.py and re-install the environment (because I have seen that issue with env issues). I ran the test a few days ago (specifically the GUNW) and it ran to completion (still failed on Circle CI). I don't know what that sig kill means in this context. I have seen that when gdal is not installed correctly or there is an issue with the environment. However, don't know. The integration tests for HRRR take a significant time to run and maybe require lots of memory.

@bbuzz31
Copy link
Collaborator Author

bbuzz31 commented Jul 27, 2023

Thanks yeah I think it has to be something weird with the memory. If I just run with HRRR (and not GMAO) it works.

@bbuzz31 bbuzz31 changed the base branch from dev to Update-dem-urls July 27, 2023 22:41
@bbuzz31 bbuzz31 changed the base branch from Update-dem-urls to dev July 27, 2023 22:41
@bbuzz31 bbuzz31 merged commit 46237d1 into dbekaert:dev Jul 28, 2023
3 checks passed
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.

[BUG] fix era5 pressure levels
4 participants