-
Notifications
You must be signed in to change notification settings - Fork 208
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
SimulationControl choices not translating to E+ #1472
Comments
@MatthewSteen Well there are some overrides in the translator to make sure simulations don't crash. This is a great example of OpenStudio trying to thread a needle between usability and full EnergyPlus power. Did we miss the mark? |
@MatthewSteen maybe I should be more explicit. If you said no to zone sizing (and there are zones and design days) your simulation would fail anyway. It has been a while since I have been in these dark corners, but I'm pretty sure it has to do with how we define ventilation. Maybe other things too. |
OK. I reviewed the Unmet Hours post. I see LSurf points has a valid scenario where zone sizing could be off and OpenStudio is not allowing it. Seems like a valid complaint. Maybe that override logic should just go away. OS default zone sizing on, allow it to be turned off manually and respect that choice potential E+ failure or not. |
OpenStudio/src/energyplus/ForwardTranslator/ForwardTranslateSimulationControl.cpp Lines 63 to 87 in c018cb6
@kbenne how about: if (modelObject.doZoneSizingCalculation()) {
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "Yes");
- } else if ((numSizingPeriods > 0) && (modelObject.model().getConcreteModelObjects<ThermalZone>().size() > 0)) {
+ } else if (modelObject.isDoZoneSizingCalculationDefaulted() &&
(numSizingPeriods > 0) && (modelObject.model().getConcreteModelObjects<ThermalZone>().size() > 0)) {
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "Yes");
} else {
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "No");
} |
Or maybe even better: #include <algorithm>
auto hasZoneEquipment = [](const Model& m) {
auto zones = m.getConcreteModelObjects<ThermalZone>();
return std::any_of(zones.cbegin(), zones.cend(), [](const auto& z){ return !z.equipment().empty(); });
};
if (modelObject.doZoneSizingCalculation()) {
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "Yes");
} else if ((numSizingPeriods > 0) && hasZoneEquipment(modelObject.model())) {
if (modelObject.isDoZoneSizingCalculationDefaulted()) {
LOG(Info, "You have zonal equipment and design days, and SimulationControl::DoZoneSizingCalculation is defaulted: turning on Do Zone Sizing Calculation");
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "Yes");
} else {
LOG(Info, "You have zonal equipment and design days, it's possible you should enable SimulationControl::DoZoneSizingCalculation");
}
} else {
simCon.setString(openstudio::SimulationControlFields::DoZoneSizingCalculation, "No");
} |
@jmarrec What is the value of DoZoneSizingCalculation for a new model. If is isn't "Yes" then won't there always be some logging noise unless the user deliberately sets "Yes"? |
Well assuming there is zone equipment... |
DoZoneSizingCalculation is useful if you have a zone with zonal equipment that is actually autosized. We don't have a handy way of checking if an equipment is autosized (we have a way to autosize() only), so that's just trying to be more precise. As far as logging goes, we can remove the case where we default it to Yes. I doubt many people read Info log levels anyways. |
@kbenne ping. |
@kbenne pong. |
@kbenne sliced ping. |
@kbenne PONG. Related to openstudiocoalition/OpenStudioApplication#693 where I realized that it'd be better is that logic would be in the model namespace rather than the ForwardTranslator. |
Fix #1472 - Respect user's SimulationControl choices and move smart defaults to the modelObject
Looks like the choices from SimulationControl aren't being translated to E+ correctly (ref: UnmetHours).
The text was updated successfully, but these errors were encountered: