-
Notifications
You must be signed in to change notification settings - Fork 4
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
Auto assign basin projection attributes and basin center #154
Conversation
…all lat log references to the topo class, retrieving zone number from the topo.nc, added a northern hemisphere config option to avoid rewriting topos files since they dont have the zone number. Removed a couple deprecated recipes relating to topo
…all lat log references to the topo class, retrieving zone number from the topo.nc, added a northern hemisphere config option to avoid rewriting topos files since they dont have the zone number. Removed a couple deprecated recipes relating to topo
…all lat log references to the topo class, retrieving zone number from the topo.nc, added a northern hemisphere config option to avoid rewriting topos files since they dont have the zone number. Removed a couple deprecated recipes relating to topo
…all lat log references to the topo class, retrieving zone number from the topo.nc, added a northern hemisphere config option to avoid rewriting topos files since they dont have the zone number. Removed a couple deprecated recipes relating to topo
The builds wont pass here until we adjust the gold files. I confirmed the failures are due to net solar in the local_hrrr and the normal RME run. |
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.
Looks good, mainly organizational changes that are requested.
…ass, moved topo tests to its own file test_topo, removed all the class attributes in loadGrid so they directly refer to the topo class, added a test to insure these attributes stay.
Ok I finally have the gold analysis. I took a little side journey to add to Joes gold diff script. I just always assumed you couldn't switch branches like that while running your code. It turns out to be a super useful for thing for me in a quite a few other areas. So I have added it all to a package called goldmeister in our group. As for my changes here you go. The new calculation for the basin lat and long is slightly different than what we see in the config file. So there will be net solar changes. Below are my changes to the gold files as the gold config files for hrrr and station data are committed currently. projections_updates >> tests/RME/gold/gold_config.ini projections_updates >> tests/RME/gold_hrrr/gold_config.ini Note, included here are the other changes reported in addition to net_solar but I believe these are the discrepancies we have seen from computer to computer for HRRR data. I wanted to be sure in my changes so I went back to the master branch and changed the config file to the lat long that I am calculating to see the changes were identical. master >> tests/RME/gold/gold_config.ini [MODIFIED] |
It appears that changes to |
@scotthavens yep, that was my interpretation. The hrrr tests when not on the host machine that made them only tests to 0.003 precision. I just didn't know what the protocol is for this situation. I almost left them out but I wanted to show them incase someone disagreed with them being float precision. |
Actually now that I say this, I am assuming you made the gold images for hrrr and you machine is the host? If thats the case and we want to keep that, we will want you to commit the gold file changes when we decide to do so. |
@jomey was the last one to change the gold files I believe but this is just a problem that we will have to figure out. To take host machines out of it, maybe we build the gold files with the docker image? |
I used the docker image to rebuild the gold files. |
Looks a lot better with just |
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.
Looks good to me, only the one very minor possible cleanup.
PR USDA-ARS-NWRC/smrf/pull/154 removed the basin lat and lon from the configuration file. These changes reflect the necessary updates to AWSM.
In #100 and #97 we are looking to remove config file options in favor of obtaining them from either the topo.nc file or from auto calculation specifically relating to the basin's center (lat lon) and the projection attributes (zone number letter etc).
I added a simple test confirming the lat lon calculations as well to test_load_data.py
This will impact the net_radiation gold file as the new lat lon is slightly different. So review the code and then I will do the analysis showing the changes to the gold files.