w2w icon indicating copy to clipboard operation
w2w copied to clipboard

Added treatment of USGS LU classes

Open lpilz opened this issue 1 year ago • 4 comments

This PR adds the capability of using 24-category USGS class data to W2W.

I don't have any tests for it yet, as I wanted to ask whether you could create a test dataset in line with the other ones for me.

lpilz avatar Oct 31 '22 19:10 lpilz

Hey 👋 thanks for your PR - while I am not the right person to review the WRF details, I may be able to point you at the testing/5by5_20cat.nc. You can maybe use this as a template to add your additional classes and write some tests specific to the added feature. Right now it seems that the NotImplementedError is the cause for at least 4 failing tests, you may want to have a look at this first and check the conditions. @matthiasdemuzere or @dargueso will have a look at the WRF-implementation details in a bit

thank you!

jkittner avatar Nov 01 '22 00:11 jkittner

Hey, thanks for the quick response - I fixed the broken tests. Just wanted to ask whether you generated the test-files using WPS or manually. And if you use WPS, could you provide me with a namelist? :)

lpilz avatar Nov 01 '22 17:11 lpilz

Hi @lpilz, Thanks for the PR. I'm having a look at your changes. The part where the codes discern which input data is used through the num_land_cat can be improved. This is is not your fault, the original code was probably too rigid and specific. I think we can use attributes in the geo_em files such as ISURBAN, ISLAKE or ISWATER, which define number for categories we use. Could you please send me your geo_em files so I can test with USGS too. That would also be useful to test why the code stalls for domain 1.

If we cannot use these attributes for whatever reason, I think we should better look into MMINLU to define the input data and get rid of all the hard-coded numbers for categories.

dargueso avatar Nov 02 '22 20:11 dargueso

Hi @dargueso, thanks for your response. I agree that the discernment is rather crude on my part and can definitely be improved. If you want, I can take a look at adding this to the PR. Please find my geo_em files attached. However, I'm not using the default USGS data, I'm using CORINE recategorized to USGS (https://zenodo.org/record/4432128).

geo_em.d01.nc.gz geo_em.d02.nc.gz geo_em.d03.nc.gz

lpilz avatar Nov 02 '22 21:11 lpilz

Hi @dargueso, thanks for your response. I agree that the discernment is rather crude on my part and can definitely be improved. If you want, I can take a look at adding this to the PR. Please find my geo_em files attached. However, I'm not using the default USGS data, I'm using CORINE recategorized to USGS (https://zenodo.org/record/4432128).

geo_em.d01.nc.gz geo_em.d02.nc.gz geo_em.d03.nc.gz

I will look at the stalling issue in the next couple of days

dargueso avatar Nov 02 '22 21:11 dargueso