Closed jomey closed 4 years ago
This should be merged before PR#65 to make tests pass again independently from the changes in that PR.
Does anyone have an idea on why the tests pass on my local with all changes, but not on Travis? I suspect it has something to do with Docker not pulling the latest from SMRF master and I can't find why that might be.
I had the same thing a couple months ago and it was due to pandas 1.0 being used on travis but not locally. Not sure if thats your issue but it might be a lead. Super frustrating though!
I should say that my experience was with SMRF not AWSM.
I had the same thing a couple months ago and it was due to pandas 1.0 being used on travis but not locally. Not sure if thats your issue but it might be a lead. Super frustrating though!
I think you are on the money here. SnowAV and Tablizer have a set version of pandas of 0.22
and that causes a downgrade on Travis. That version however does not have the to_list()
method.
Can we relax the requirement for those modules or have them optional?
@robertson-mark are we able to relax the snowav and tabilizer reqs?
I had the same thing a couple months ago and it was due to pandas 1.0 being used on travis but not locally. Not sure if thats your issue but it might be a lead. Super frustrating though!
I think you are on the money here. SnowAV and Tablizer have a set version of pandas of
0.22
and that causes a downgrade on Travis. That version however does not have theto_list()
method.Can we relax the requirement for those modules or have them optional?
Potentially related: https://github.com/USDA-ARS-NWRC/smrf/pull/159#pullrequestreview-407013111
Here the log excerpt from Travis that made me suspect this:
tablizer 0.2.2 has requirement numpy==1.16.3, but you'll have numpy 1.16.2 which is incompatible.
tablizer 0.2.2 has requirement pandas==0.22.0, but you'll have pandas 1.0.3 which is incompatible.
snowav 0.11.16 has requirement numpy==1.16.3, but you'll have numpy 1.16.2 which is incompatible.
snowav 0.11.16 has requirement pandas==0.22.0, but you'll have pandas 1.0.3 which is incompatible.
How many places is pandas being pinned? Most likely all the repos except for SMRF now, but we may need to make the effort to get everything to use >1.0? There is still a lot of back compatibility and it's only some of the odd functions like to_list()
@robertson-mark Can we just remove snowav and tablilzer from AWSM? For the real time simulations, snowav is running independently anyways meaning that it doesn't really need AWSM or is it too tightly coupled?
@scotthavens Yeah, removing snowav from awsm would be fine for the current usage. We haven't used them coupled for ops runs in some time. The test cases that you were working on required them though, if I'm not mistaken - so as long as we sort that out. I can also look at relaxing the pandas requirements, but you'll have to let me know how that fits in with wrap-up tasks.
@jomey lets just remove snowav as a requirement from AWSM if everyone is okay with that, it's a fairly specialized case for the operational runs. In the end, I'd like to see snowav in it's own docker image that is fully decoupled from AWSM.
Removing snowav would also slim down the docker image for AWSM immensely because LaTex isn't needed.
We will need to update awsm_test_cases, just want to get that out there
The awsm test cases have a pinned AWSM version since it uses the docker. It was not meant for a general test case, just specific for the manuscript.
I will look into removing the two dependencies and add it to this PR.
@scotthavens - I am getting a new error with the changes to wind distribution in SMRF. This is with the latest changes in master that got merged on Friday.
Here the stack trace:
ERROR: test_smrf_pysnobal_single (tests.test_model.TestModel)
Test smrf passing variables to PySnobal
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Volumes/warehouse/projects/ARS/awsm/tests/test_model.py", line 103, in test_smrf_pysnobal_single
self.assertIsNone(run_awsm(config))
File "/Volumes/warehouse/projects/ARS/awsm/awsm/framework/framework.py", line 815, in run_awsm
a.run_smrf_ipysnobal()
File "/Volumes/warehouse/projects/ARS/awsm/awsm/framework/framework.py", line 414, in run_smrf_ipysnobal
smrf_ipy.run_smrf_ipysnobal(self)
File "/Volumes/warehouse/projects/ARS/awsm/awsm/interface/smrf_ipysnobal.py", line 159, in run_smrf_ipysnobal
run_smrf_ipysnobal_single(myawsm, s)
File "/Volumes/warehouse/projects/ARS/awsm/awsm/interface/smrf_ipysnobal.py", line 273, in run_smrf_ipysnobal_single
s.distribute['wind'].dir_round_cell,
AttributeError: 'Wind' object has no attribute 'dir_round_cell'
The threaded tests pass and the single won't. I looked into the SMRF changes, but don't know what the proper solution would be for non-threaded runs.
It will look like s.distribute['wind'].wind_model.dir_round_cell
as the wind_model
object contains the class for distributing each of the wind model methods.
I am done from my end and ready for another round of review.
I know in the SMRF basin lat/lon PR, the gold files changed slightly. Was that not the case for AWSM or is the basin lat/lon already the center of the topo?
Passed both on my local and on Travis without re-generating those
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.
Call it a mini scope creep and
snowav
is completely removed now including the latex package from the Docker.Also messed with the ASCII art and it looks this way now: