Conversation
|
To do:
To consider:
|
|
@jfrederik-nrel , to keep things clear between this PR and #232, how about I remove the |
|
@dzalkind @jfrederik-nrel @genevievestarke this PR is now ready for review. |
|
Move registry to a different place in Hercules? |
|
|
||
| import copy | ||
|
|
||
| import hercules.hybrid_plant as hp |
There was a problem hiding this comment.
@genevievestarke , does your comment about moving the registry refer to changing this line so we don't have to import hybrid_plant here? I tried that for a while but kept running into circular import issues so gave up. It sounds like @paulf81 also has had this issue before. I do agree though that this seems weird, and it would be better if the registry was in its own file. Perhaps we create a separate issue for that?
There was a problem hiding this comment.
Yeah, @dzalkind mentioned that you were looking for a fix, but we didn't think there was one implemented yet. I'll make an issue!
There was a problem hiding this comment.
See if moving this to a new file is useful.
genevievestarke
left a comment
genevievestarke
left a comment
There was a problem hiding this comment.
Looks good, I just left some comments so I remember which edits I wanted. I'll be working on applying these today.
|
|
||
| import copy | ||
|
|
||
| import hercules.hybrid_plant as hp |
There was a problem hiding this comment.
See if moving this to a new file is useful.
|
|
||
| # Load component registry here to define units in thermal plant | ||
| # NOTE: this breaks a circular dependency issue | ||
| from hercules.component_registry import COMPONENT_REGISTRY |
There was a problem hiding this comment.
@paulf81 Thoughts on moving the component registry to a new file and adding this import statement later in thermal_plant?
4 participants
This pull request adds functionality for having multiple thermal generating units under a single thermal power plant. This could be several independent units (e.g. two open-cycle gas turbines) or dependent units, such as the gas turbine and the steam turbine in a combined cycle gas plant. The latter is not currently included, although @jfrederik-nrel is working on it.
As part of this PR, @jfrederik-nrel has also added a generic steam turbine unit. Final validation and documentation of this will be added before we mark this PR as ready for review.
EDIT: as per discussion below, the
SteamTurbineclass was removed from this PR and moved to #232 instead.