-
Notifications
You must be signed in to change notification settings - Fork 394
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
Add missing references for support of FanSystemModel in various HVAC/ZoneHVAC components #7721
Conversation
…ings work: missing plant equipment references etc Note that for solar collectors, technically E+ does say it expects them on the demand side, but it works on the supply... NREL/OpenStudio#3762 NREL/OpenStudio#3761 (comment) NREL/OpenStudio#2880
… and PlantEquipmentList nowadays
…:COmponentModel, so update ref to more restrictive than "Fans"
…as well( checked OutdorAirUnit.cc)
… Fan:COmponentModel actually...
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.
@jmarrec Not sure if this was ready for review yet, but made some comments anyway.
\reference-class-name validPlantEquipmentTypes | ||
\reference validPlantEquipmentNames | ||
\reference-class-name validCondenserEquipmentTypes | ||
\reference validCondenserEquipmentNames |
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 don't think this is correct. Generators with waste heat recovery can sit on a branch, but they are not dispatchable by the load management. So, I don't think they belong in plant or condenser equipment lists. @Myoldmopar ?
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.
If you use an ElectricLoadCenter:Distribution
object with a scheme like FollowThermalLimitElectric
, then you do have to put in a PlantEquipmentOperation:HeatingLoad
don't you?
That's what I'm doing in the OpenStudio-resources regression test for Generator:MicroTurbine at least... https://github.com/NREL/OpenStudio-resources/blob/9d16ac6aba234794c84438a679ce4ff54710dcc5/model/simulationtests/generator_microturbine.rb#L385
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 not really sure if that does anything. You may be correct. @Myoldmopar @EnergyArchmage ?
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 happen to be working with the Generator:MicroTurbine by adding it to the MediumOffice reference building (using Julien's generator_microturbine.rb
code, thanks btw!), and can confirm that the PlantEquipmentOperation:HeatingLoad
is needed for the ElectricLoadCenter:Distribution
> FollowThermalLimitElectrical
case to have any effect on a model.
@@ -68354,6 +68382,8 @@ SolarCollector:FlatPlate:Water, | |||
\type alpha | |||
\reference-class-name validBranchEquipmentTypes | |||
\reference validBranchEquipmentNames | |||
\reference-class-name validPlantEquipmentTypes | |||
\reference validPlantEquipmentNames |
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.
Not sure solar collectors belong on PlantEquipmentLists? @mitchute @Myoldmopar ?
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.
Looks to me like it should be there:
EnergyPlus/src/EnergyPlus/SolarCollectors.hh
Line 128 in bcfdd25
struct CollectorData : PlantComponent |
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.
Lots of confusion on what the placement of solar collectors should be, especially since OpenStudio forces you to put the Flat Plate ones on the supply side (for some reason... and it appears to work fine too...)
I agree that generally speaking the documentation and example files in E+ seem to imply demand side placement (with control of the loop via availability managers).
References:
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.
Opening a dedicated issue for this instead: #7735
It is possible for a chp generator to be operated in a thermal load following scheme. I think @jmarrec is correct. Just glanced at comments. |
in (Note that I added the reference for Generator:MicroTurbine but not for Generator:MicroCHP simply because I modified that stuff while testing things that are implemented in OS, and MicroTurbine is but MicroCHP isn't. Edit: actually MicroCHP already has the same references) |
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.
@jmarrec I'm ok with the responses to prior comments. I've tested this with IDF Editor validity check for about 2 dozen idf files that contain Fan:SystemModel. All passed without errors. I've updated to current develop and will wait for one more round of CI results, then merge this.
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.