-
Notifications
You must be signed in to change notification settings - Fork 195
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 Fan:SystemModel to OpenStudio #3856
Conversation
…t in Ctor (see details below:) ``` setAvailabilitySchedule(model.alwaysOnDiscreteSchedule()); autosizeDesignMaximumAirFlowRate(); setDesignPressureRise("????"); setSpeedControlMethod("Discrete"); setElectricPowerMinimumFlowRateFraction(0.2); setMotorEfficiency(0.9); setMotorInAirStreamFraction(1.0); autosizeDesignElectricPowerConsumption(); setElectricPowerPerUnitFlowRatePerUnitPressure(1.66667); => setDesignPowerSizingMethod("PowerPerFlowPerPressure"); setFanTotalEfficiency(0.7); setNumberofSpeeds(1); ```
… they can fail or not
Probably needs cleanup, deal with output directory, etc
…d (this one has a default), and the Electric Power Per Unit Flow Rate too (this one I'll create a default)
…e are going to fail, like AirLoopHVACUNitaryHeatPumpxx that doesn't support it), clone,
…ZoneHVACComponent: WIP (too many types!)
… per pressure) times the 500 Pa I set for design pressure rise
… reevaluated: partial classes in C#, forward decl in ruby)
…number of extensible groups. Also make the Speed X FLow Fraction a require-field (not the other... advanced use only via pushExtensibleGroup)
KB: Strongly agree with your decision. I also try to reduce this kind of redundancy when I see it.
KB: This seems ok to me. In general I like not needing to have a decision tree diagram to figure out what is required input data. Does this choice avoid any optional fields in the API? If so then I am doubly in favor.
KB: Yeah and if we do try to implement rules within OpenStudio we will probably fail in keeping them in sync with EnergyPlus over time. Also if you were to implement these rules in the model API (as opposed to the FT) how would you communicate the dynamically required/optional fields as related field change? I think your choice is the correct one. Let EnergyPlus report any input errors in this case.
KB: My first reaction is follow the EnergyPlus IDD even if it is wrong. Consider a PR to EnergyPlus instead, and then update OpenStudio IDD if EnergyPlus merges. Don't you think that even if you correct the OpenStudio IDD you might still have a failed simulation when EnergyPlus can't validate against its IDD? |
Pull request overview
NOTES
FanOnOff
while a bunch of ZoneHVAC objects accept Type= Fan:SystemModel but Name only FanOnOff. Cf Fan:SystemModel inconsistencies in IDD EnergyPlus#7697Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
): N/Asrc/openstudio_lib/library/OpenStudioPolicy.xml
): Add Fan:SystemModel to OpenStudioApplication openstudiocoalition/OpenStudioApplication#72Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.