search

noaa-oar-arl / canopy-app

Stand-alone/column canopy codes and parameterizations
MIT License
6 stars 6 forks source link

Update ch and canfrac #109

Closed angehung5 closed 5 months ago

angehung5 commented 5 months ago

Based on #106

Example files are updated:

README data table is also updated but I leave the thresholds to you.

drnimbusrain commented 5 months ago

@angehung5 great start thanks! As discussed, will wait until all code/script/file changes have been made and the new variables are consistent for canopy height (ch) and canopy fraction (canfrac) instead of "forest" only related names/variables. This will also include changes to input canopy thresholds controlled in namelist, fch_thresh and frt_thresh, which should become something like "ch_thres" and "cf_thresh", respectively. This may impact CI and testing features, @zmoon ?

angehung5 commented 5 months ago

@drnimbusrain @zmoon OK, I changed "fh/fch", "ffrac", "fch_thresh", and "frt_thresh" to "ch", "canfrac", "ch_thresh", and "cf_thresh", respectively. Code checks passed and there is no old var names in the code, but please check again in case I miss anything.

drnimbusrain commented 5 months ago

@angehung5 Also, since there are NL changes to the thresholds, as mentioned I think there also needs to be changed in Zach's example jupyter notebook: https://github.com/angehung5/canopy-app/blob/main/python/examples.ipynb

zmoon commented 5 months ago

Thanks @drnimbusrain

I see that although for 30eea0b there was an error when running the nb, it didn't fail CI. I should fix that. Probably need to check the rc in the loop and exit if nonzero.

drnimbusrain commented 5 months ago

Thanks @drnimbusrain

I see that although for 30eea0b there was an error when running the nb, it didn't fail CI. I should fix that. Probably need to check the rc in the loop and exit if nonzero.

@zmoon Thanks for checking this! Are we good to go for now though as @angehung5 made the changes to the examples nb, or do you want to address that CI check here?

angehung5 commented 5 months ago

Looks good @angehung5 , and I think this covers all instances. I am uploading the new AWS files now. Can you run a test on the default slurm script for the global data process and run to make sure all works and looks OK for both scripts and canopy-app run and outputs?

Updated code works fine with southeast US files, and the slurm script and global python script work as well. Outputs look good.

drnimbusrain commented 5 months ago

Thanks @angehung5