-
Notifications
You must be signed in to change notification settings - Fork 37
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
Work on making a pip installable HERON package. #216
Conversation
Requires idaholab/raven#1994 |
with warnings.catch_warnings(): | ||
warnings.simplefilter('ignore') | ||
rom = ravenROM(fpath, raven_loc).rom | ||
try: |
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.
I found that this method gets called repeatedly in the DispatchManager.py
for each ARMA realization. I was able to reduce quite a significant amount of memory consumption by memoizing this function. Instead of making a whole new PR, it might be worth to just add here. All you need to do is:
at the top in the imports:
from functools import cache
and then add the decorator to the function:
@cache
get_synthhist_structure
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.
ohhh, because RAVEN runs this each time instead of once ever ... maybe Mohammad's work will fix this to some degree. Good catch.
Does cacheing fix this if parallelized?
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.
Thanks, fixed: 764ce1c
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.
I am pretty should this cache is per process, so non-shared memory parallelism would have to cache it multiple times.
Job CentOS 8 on 764ce1c : invalidated by @joshua-cogliati-inl Updated RAVEN to add ravenframework.ROMExternal |
src/dispatch/pyomo_dispatch.py
Outdated
@@ -21,7 +21,8 @@ | |||
# allows pyomo to solve on threaded processes | |||
import pyutilib.subprocess.GlobalData | |||
pyutilib.subprocess.GlobalData.DEFINE_SIGNAL_HANDLERS_DEFAULT = False | |||
from pyutilib.common._exceptions import ApplicationError | |||
#from pyutilib.common._exceptions import ApplicationError |
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.
Commented code can be removed.
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.
Just one change and one question. Otherwise changes look good to me.
@@ -106,7 +106,7 @@ | |||
</Components> | |||
|
|||
<DataGenerators> | |||
<ARMA name='Price' variable="Signal">%HERON%/tests/integration_tests/ARMA/Sine/arma.pk</ARMA> | |||
<ARMA name='Price' variable="Signal">%BASE_WORKING_DIR%/../../../ARMA/Sine/arma.pk</ARMA> |
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.
Curious about the purpose this change.
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.
Basically, this is for testing heron from a pip install, and in a pip install, %HERON% may not be anywhere near the tests directory, so I changed it to be relative to the base working directory. At some point all the tests will have to be changed to do this.
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.
So after HERON is pip installed, we can:
cd tests/integration_tests/workflows/production_flex
heron heron_input.xml
raven_framework outer.xml
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.
Changes look good and don't affect major functionality.
Pull Request Description
What issue does this change request address?
idaholab/raven#1764
What are the significant changes in functionality due to this change request?
Allow running parts of HERON in pip installed mode.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.