Conversation
| # This sets up the state entries for all state compatible HSP2 model variables | ||
| # print("STATE initializing contexts.") | ||
| for _, operation, segment, delt in opseq.itertuples(): | ||
| if operation != "GENER" and operation != "COPY": |
There was a problem hiding this comment.
This should be:
if operation not in ["GENER", "COPY", "PLTGEN", "DISPLY"]:
Or maybe (I think these are the operations that apply here, but ?):
if operation in ["RCHRES", "PERLND", "IMPLND"]:
I think this is the current test failure.
I read in "PLTGEN" and "DISPLY" operations now, even though unused by HSP2. My idea was to have complete interoperability between parameter stores (HDF5, UCI, and CSV).
|
Thanks @timcera -- checking this out now! |
|
Thanks @timcera -- this definitely fixed my initial problem, though now I have some results differences that I have to sort out, I assume they are unrelated to the improvements that you've made to the UCI parser. |
|
@timcera - I went back to basics on this to try and track my errors. I am having trouble importing UCI's with the latest An I get |
|
Sorry that was me. Fixed in #229. |
|
Thanks @timcera for the quick fix! Any clue how this passed the tests on the |
|
The tests passed because they use a directory path to the UCI file, something like "tests/test10/HSP2results/test10.uci" instead of "test10.uci". When given "test10.uci" when you are in the directory the previous code found the parent directory as "" which meant that when it went to look for the WDM files (which are relative paths to the UCI) was looking for "/test10.wdm" instead of "test10.wdm". The pathlib Path function handles all of that without problem, and supports cross-platform path manipulations. I don't know why I didn't use it before. |
|
The fix in #229 isn't the cause of these test failures. I don't know what is causing this issue. Could it be something different in the way I am parsing and creating the Special Action tables that is different than what you were working with? |
|
Thanks @timcera that makes sense! The tests are still failing now, but in the value comparison phase. Which is equally mystifying, but at least it's something! The tests in question all seem to be related to the differences in sediment induced by the special actions tests, so, that makes me think I broke something :). Even though the same code previously passed, perhaps I goofed something up when bringing that older code into the more recent Thanks again! |
|
If you find that it is something in the UCI parsing code, I would be glad to handle the fix. |
|
@timcera -- apologies for my delay in getting back to you, your suggested fix did the trick I had hoped that the generic @PaulDudaRESPEC - this PR is ready to merge unless you or Tim have reservations. It adds very little, but streamlines the code a bit, and I think that will facilitate better readability in the short term, and easier migrateability to the new looping approach when we get there. |
Replaces #224 after integrating recent UCI changes
This PR includes an overhaul to the model
statesystem, leveraging a new object-oriented version ofstate, which accomplishes the following:statetakes the place ofstate_ix,dict_ix,ts_ix, ...tests/testcbp/HSP2results/to test a 500 equation addon simulation (Use:cp PL3_5250_0001.json.manyeq PL3_5250_0001.jsonto test).hydr()andsedtrn()to eliminate the argumentio_manageras this is no longer used (I think this has been due for a while, I don't think it is due to any changes that I recall making to the code but it seems like a cleaner approach)