-
Notifications
You must be signed in to change notification settings - Fork 133
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
OutStreams as RAVEN entity #1329
Conversation
from .FilePrint import FilePrint | ||
from .GeneralPlot import GeneralPlot | ||
# from .DataMining import DataMining | ||
# from .VDCComparison import VDCComparison |
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.
These are placeholders for the future; they can be removed if desired. Same with later on in this file.
Because of that "version 5.7.0" pyomo issue, adding the HERON submodule update as well. The PR I wanted has been merged, anyway. |
I didn't consider the possibility of a PIP-only install with HERON; I've added it to HERON feature requests: idaholab/HERON/issues/33 |
'Print': FilePrint, | ||
'Plot': GeneralPlot, | ||
# 'DataMining': DataMining, | ||
# 'VDCComparison': VDCComparison, |
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.
In my opinion the specializations of the Plots/Prints should be under Plot/Print (similarly to External Model/type or Code, CodeInterfaces)
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.
placeholder for discussion
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'm noticing immediately that there would be a big benefit for a base class for Plot
, although I think the Entity approach is cleaner. Maybe in the future:
<OutStreams>
<Print>
<MyCustomPrint>
<... options ...>
</MyCustomPrint>
</Print>
</OutStreams>
although there's a couple things I don't love about this:
- It forces an OutStream to be only a Print or a Plot (I can imagine cases where both make sense)
- It is regressing towards the approach in the listing
class=OutStream, type=Print, subType=MyPrint
instead ofclass=OutStream type=MyPrint
- We have to modify all existing RAVEN inputs if we add a subspot under for , or some fancy checking to see if we're dealing with a special type or the generic type.
The other two options I see are the way I hinted at here
<OutStreams>
<MyCustomPrint>
<... options ...>
</MyCustomPrint>
</OutStreams>
or using the subType
approach from the ROMs, that I think we don't like much.
Definitely a good discussion to have when we start designing the first new entries.
Minor changes |
is the HERON update done on purpose? or just a mistake? |
okay. it is done on purpose. Asked @PaulTalbot-INL |
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.
comments addressed
PR is approved to be merged. |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Closes #1328
Addresses #1114, Closes #1325
What are the significant changes in functionality due to this change request?
Restructures OutStreamManager, OutStreamPrint, and OutStreamPlot.
New base class is OutStreamBase, which is a standard RAVEN entity.
New printer is
FilePrint
, which is accessed through the<Print>
option for traditional printing.New plotter is
GeneralPlot
, which is accessed through the<Plot>
option for traditional plotting.New entities for OutStreams can inherit from OutStreamBase and be added to the Factory to create new options.
No change to existing input files is needed as a result of these changes.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True.raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example.