Dove modifications #387
Open
Dove modifications #387
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dylanjm
force-pushed
the
dove-modifications
branch
2 times, most recently
from
531c675 to
84bc649
Compare
dylanjm
force-pushed
the
dove-modifications
branch
from
8e89003 to
860876c
Compare
dylanjm
force-pushed
the
dove-modifications
branch
from
1552aaa to
731c206
Compare
dylanjm
force-pushed
the
dove-modifications
branch
from
731c206 to
bb1707c
Compare
Collaborator
PaulTalbot-INL
left a comment
PaulTalbot-INL
left a comment
There was a problem hiding this comment.
Initial surface review, very few comments. I did not comment on many commented lines that might be removed; I'll save that for a future review.
| ## All InputSpecs for Interactions are defined within the DOVE.src.Interactions. | ||
| ## If a new VP node needs to be added, find someway to add a fixed-value version | ||
| ## to DOVE first! Then modify it in here. This will keep feature parity with DOVE. | ||
| ## YOU HAVE BEEN WARNED! |
| sub.addSub(new_sub) | ||
|
|
||
| # We have to iterate a level deeper to get to CashFlow nodes | ||
| for sub in input_specs.subs: |
Collaborator
There was a problem hiding this comment.
can this be merged with the loop above by keeping the econ "if" as the first if? Probably not necessary or even very impactful but may save a little computational effort
| # when recreating the ValuedParam version of <reference_price>. Otherwise it gets deleted and is no | ||
| # longer seen as an available input option. We should find a way to dynamically add child subs by perhaps | ||
| # adding a key to `econ_subs_to_modify` dictionary like "add_subs". For now we add a conditional checking | ||
| # if we are modifying <reference_price> and then directly add <levelized_cost> to the new input definition. |
Collaborator
There was a problem hiding this comment.
10 laps for you. I agree, we need a cleaner way to do this at some point.
src/Components.py
Outdated
| @ Out, None | ||
| """ | ||
| Interaction.__init__(self, **kwargs) | ||
| HeronInteraction.__init__(self, **kwargs) |
Collaborator
There was a problem hiding this comment.
do we not need __init__ from DoveProducer here as well?
dylanjm
force-pushed
the
dove-modifications
branch
2 times, most recently
from
6403be3 to
691cca3
Compare
dylanjm
force-pushed
the
dove-modifications
branch
from
691cca3 to
08b3b4d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
What issue does this change request address?
#386
What are the significant changes in functionality due to this change request?
Ideally, no changes in functionality but everything will be different.
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.