Skip to content

Conversation

@jmarrec
Copy link
Contributor

@jmarrec jmarrec commented May 22, 2025

Pull request overview

Description of the purpose of this PR

Pull Request Author

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

@jmarrec jmarrec requested a review from mjwitte May 22, 2025 10:53
@jmarrec jmarrec self-assigned this May 22, 2025
@jmarrec jmarrec added Defect Includes code to repair a defect in EnergyPlus DoNotPublish Includes changes that shouldn't be reported in the changelog labels May 22, 2025
Comment on lines +2739 to +2770
auto writeDSOAToPredefined = [&state, &vent_mech, &thisMechVentZone](const OARequirementsData &dsoa,
const std::string &zoneOrSpaceName) {
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVventMechName, zoneOrSpaceName, vent_mech.Name);
OutputReportPredefined::PreDefTableEntry(state,
state.dataOutRptPredefined->pdchDCVType,
zoneOrSpaceName,
SysOAMethodNames[static_cast<int>(vent_mech.SystemOAMethod)]);
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVperPerson, zoneOrSpaceName, dsoa.OAFlowPerPerson, 6);
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVType, zsName, SysOAMethodNames[static_cast<int>(vent_mech.SystemOAMethod)]);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVperPerson, zsName, dsoa2.OAFlowPerPerson, 6);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVperArea, zsName, dsoa2.OAFlowPerArea, 6);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVperZone, zsName, dsoa2.OAFlowPerZone, 6);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVperACH, zsName, dsoa2.OAFlowACH, 6);
state, state.dataOutRptPredefined->pdchDCVperArea, zoneOrSpaceName, dsoa.OAFlowPerArea, 6);
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVMethod, zsName, OAFlowCalcMethodNames[static_cast<int>(dsoa2.OAFlowMethod)]);
if (dsoa2.oaFlowFracSched != nullptr) {
state, state.dataOutRptPredefined->pdchDCVperZone, zoneOrSpaceName, dsoa.OAFlowPerZone, 6);
OutputReportPredefined::PreDefTableEntry(state, state.dataOutRptPredefined->pdchDCVperACH, zoneOrSpaceName, dsoa.OAFlowACH, 6);
OutputReportPredefined::PreDefTableEntry(state,
state.dataOutRptPredefined->pdchDCVMethod,
zoneOrSpaceName,
OAFlowCalcMethodNames[static_cast<int>(dsoa.OAFlowMethod)]);
if (dsoa.oaFlowFracSched != nullptr) {
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVOASchName, zsName, dsoa2.oaFlowFracSched->Name);
state, state.dataOutRptPredefined->pdchDCVOASchName, zoneOrSpaceName, dsoa.oaFlowFracSched->Name);
}
if (thisMechVentZone.zoneADEffSched != nullptr) {
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVZoneADEffSchName, zsName, thisMechVentZone.zoneADEffSched->Name);
state, state.dataOutRptPredefined->pdchDCVZoneADEffSchName, zoneOrSpaceName, thisMechVentZone.zoneADEffSched->Name);
} else {
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVZoneADEffCooling, zsName, thisMechVentZone.ZoneADEffCooling, 2);
state, state.dataOutRptPredefined->pdchDCVZoneADEffCooling, zoneOrSpaceName, thisMechVentZone.ZoneADEffCooling, 2);
OutputReportPredefined::PreDefTableEntry(
state, state.dataOutRptPredefined->pdchDCVZoneADEffHeating, zsName, thisMechVentZone.ZoneADEffHeating, 2);
state, state.dataOutRptPredefined->pdchDCVZoneADEffHeating, zoneOrSpaceName, thisMechVentZone.ZoneADEffHeating, 2);
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lambdas for the win.

Copy link
Collaborator

@amirroth amirroth May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just make a normal function? Lambdas are for passing functions as arguments to other functions, not for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's your personal interpretation of what lambdas are, but it's not mine.

Lambdas are inline, anonymous functions, and they can take a closure, and they are (in my opinion) a great way to do what I've just did, which is to 1) fix the issue and 2) dry up the code.

Sure, you can write a function. But I don't see it being used anywhere else, and you're going to have to pass a bunch of parameters, and probably re-access thisMechVentZone. And now the logic to do stuff is further away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just explaining why I like it, you can still ask @mjwitte to do it with a function on the original PR of course :)

Copy link
Collaborator

@amirroth amirroth May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I'm not going to ask @mjwitte to do anything.

Comment on lines +2771 to +2775
if (dsoa.numDSOA == 0) {
writeDSOAToPredefined(dsoa, zoneName);
} else {
for (int n = 0; n < dsoa.numDSOA; ++n) {
writeDSOAToPredefined(state.dataSize->OARequirements(dsoa.dsoaIndexes[n]), format("{}:{}", zoneName, dsoa.dsoaSpaceNames[n]));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now it's short, sweet, and clear.

@jmarrec
Copy link
Contributor Author

jmarrec commented May 22, 2025

Alright confirmed that my MCVEs now run to completion.

Copy link
Contributor

@mjwitte mjwitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works. I'll merge this shortly and finalize #11051 for review. Thanks @jmarrec

@mjwitte mjwitte merged commit ae0f722 into NREL:DSOASpaceList May 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus DoNotPublish Includes changes that shouldn't be reported in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants