search

USDA-ARS-NWRC / smrf

SMRF was designed to increase the flexibility of taking measured weather data, or atmospheric models, and distributing the data across a watershed.
Other
12 stars 4 forks source link

Threading cleanup and testing #176

Closed scotthavens closed 4 years ago

scotthavens commented 4 years ago

Trying to tackle the larger issue #114. Added a specific Lakes test that uses gridded data and threading. There were a few things that needed to be fixed and I have moved all the thread variables to their modules for better control. Also added tests for outputting specific variables from both the threaded and non threaded case.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.4%) to 79.415% when pulling 05b6a69afbb05a221cc56e0e11784d8bc77c42a5 on scotthavens:18_threading into b8702779c428f7891b938cfc94c4df3aa3b96437 on USDA-ARS-NWRC:master.

scotthavens commented 4 years ago

There are some odd problems with the list copy for the thread variables. Might have to use the bigger deepcopy guns...

scotthavens commented 4 years ago

I think that there are some thread variables that are persisting between test runs, that was the problem I was having especially going from station runs to HRRR runs.

jomey commented 4 years ago

I think that there are some thread variables that are persisting between test runs, that was the problem I was having especially going from station runs to HRRR runs.

Curious on why that only happens on the Python 3.6 runs. Last night, I was trying to see if there were some changes in Python and could not find any. I will have another look. Locally they consistently pass with 3.8

scotthavens commented 4 years ago

It's not just the 3.6, I've seen it pass there then fail on OSX 3.8. So there is something that is persisting between the simulations in the queue

jomey commented 4 years ago

I did some more reworking of the thread variable handling https://github.com/USDA-ARS-NWRC/smrf/tree/18_threading to prevent convoluting the code with deepcopy

jomey commented 4 years ago

I think we should close one of the branches. My vote would be for your fork, so we can both update.

scotthavens commented 4 years ago

Sounds good, this one is puzzling!

scotthavens commented 4 years ago

Closing in favor of #178

jomey commented 4 years ago

I just updated the comparison for the time variable and it now passes. I am a little suspicious of that success though. Can you run it on your local for a double check?