HARPgroup / HSPsquared

Hydrologic Simulation Program Python (HSPsquared)
GNU Affero General Public License v3.0
1 stars 0 forks source link

Meet w/JK and get things libbed and strategy for using hsp2 libs also #43

Closed rburghol closed 1 year ago

rburghol commented 1 year ago
jdkleiner commented 1 year ago

I've done initial setup and testing of loading RB custom functions in HSP2.

@rburghol

rburghol commented 1 year ago

This is awesome! Can't wait to give it a rest run. I too have i'm almost ready to test code inside of specl().

rburghol commented 1 year ago

@jdkleiner -- I need some help on my attempt to install the objects. Here is the lines I've added to the top of HYDR.py in order (after SPECL):

...
from HSP2.SPECL import specl, _specl_
from HSP2.modelObject import modelObject
from HSP2.Equation import Equation, exec_eqn
...

When I try to run the model I get:

  File "/usr/local/lib/python3.8/dist-packages/HSP2/Equation.py", line 8, in <module>
    class Equation(modelObject):
NameError: name 'modelObject' is not defined

The files all exist in the HSP2 directory (listed below), and I reran the install step. Thoughts?

ls -lrth HSP2/
...
-rw-rw-r--+ 1 rob      rob       3432 Dec 19 20:53 modelObject.py
-rw-rw-r--+ 1 rob      rob       4008 Dec 19 20:53 Equation.py
-rw-rw-r--+ 1 rob      rob      10594 Dec 19 21:01 utilities_specl.py
-rw-rw-r--+ 1 rob      rob      26168 Dec 19 21:03 HYDR.py
rburghol commented 1 year ago

You wanna know how important docstrings are? The modelObject class is not recognized without one!
(saw your notes "We should get really good at using docstrings...") Also, the Equation class definition file needs to import the ModelObject class file (so it repeats the import of ModelObject that is in HSP2, or perhaps that is redundant).

And my class names and file names suck. I just checked the style guide and I am ashamed. See https://peps.python.org/pep-0008/

But if you add these 2 lines to Equation.py I bet it will go farther:

from HSP2.model_object import ModelObject
from numba import njit
jdkleiner commented 1 year ago
rburghol commented 1 year ago

This is great! I took the work that you did and built on it. Got it running with a sample set of calculations, copying withdrawals back to the simulation as O1. Haven't verified that the withdrawals are actually affecting the simulation, but will do that shortly.

Changes/responses to your questions and suggestions:

Thanks again! On to DataMatrix

rburghol commented 1 year ago

OK. Done with development myself for a few hours. If you want to play around:

jdkleiner commented 1 year ago

All great stuff, I'm going to hop in there and play around a bit!

jdkleiner commented 1 year ago

@rburghol Hey Rob, can you tweak the permissions so I don't have to sudo to edit HSP2 stuff, maybe something got goofed in the creation of the specom branch?

rburghol commented 1 year ago

Try this:

sudo chgrp modelers .git/* -R
sudo chmod g+w .git/* -R
jdkleiner commented 1 year ago

No dice. Strange, I would have thought that would do the trick!

rburghol commented 1 year ago

Can you paste the error message up here?

jdkleiner commented 1 year ago

Yeah I just get a generic permissions denied error when trying to save a file for example: image

rburghol commented 1 year ago

Ahh ok. You need to do the chmod/chgrp for all the files in the HSP2 directory -- Till then note that perms are directory specific, so the other chmod/chgrp only affects the stuff in the .git directory.

Note: I think we can resolve this once we resolve login the error: groups: cannot find name for group ID 1430770

jdkleiner commented 1 year ago

Ah that makes sense thank you!

Stashing this here so I'll remember:

jkleiner@deq3:/opt/model/HSPsquared$ sudo chgrp modelers HSP2/* -R
jkleiner@deq3:/opt/model/HSPsquared$ sudo chmod g+w HSP2/* -R
rburghol commented 1 year ago

All good. ~Hey @jdkleiner I am having trouble now since I committed your changes -- the calculations are no longer being performed in step_model(). It is wierd, it looked to me like you just changed some formatting on the print statements, so nothing should have changed, but... did you make any other changes or are there gremlins in that code now?~

Figured it out! Small change to the Qintake equation (by me by accident) and voila! Broken. Now un-broken. Thanks!

jdkleiner commented 1 year ago

@rburghol Ah very good! Was just going to say, all I tweaked were those print statements to get a better understanding of whats going on. I'm out of there now, its all yours!

rburghol commented 1 year ago

FYI @jdkleiner -- benchmarking this thing, with 100 random equation ops we get a total runtime (after initial comple) of 7.9 seconds. I added 1,000 random equations and runtime went to 10.0 seconds. Added 10,000 random equations and total time goes to 45 seconds. It appeared that the largest amount of time was spent parsing the equations, rather than executing the model. Since equation parsing should take place when we import_uci that should offer us an opportunity to keep some of that minimized.

jdkleiner commented 1 year ago

Sounds quite promising!