ConstructionProperty:InternalHeatSource not being properly integrated with the surface heat balance - #11729
Open
joseph-robertson wants to merge 29 commits into
Open
ConstructionProperty:InternalHeatSource not being properly integrated with the surface heat balance#11729joseph-robertson wants to merge 29 commits into
ConstructionProperty:InternalHeatSource not being properly integrated with the surface heat balance#11729joseph-robertson wants to merge 29 commits into
Conversation
|
…pping on other radiant systems.
joseph-robertson
commented
Aug 6, 2026
Collaborator
Author
|
Here is the before (blue) and after (orange) for Zn001:Wall001a (IntegratedSurfaceOutsideFace) of GeneratorswithPV.idf in USA_CO_Denver-Aurora: 
|
|
|
|
|
…n integrated PV present.
|
|
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #11698 by ensuring building-integrated photovoltaic (PV) source/sink terms are incorporated into the surface heat balance after they become available during the HVAC/air heat balance sequence.
Changes:
- Adds a Photovoltaics utility (
HasBuildingIntegratedPV) to detect whether any PV array uses an integrated heat transfer mode. - Updates
UpdateFinalSurfaceHeatBalanceto trigger an additional outside/inside surface heat balance pass when integrated PV is present. - Minor cleanup in
Photovoltaics::ReportPV(fully-qualifyTranspiredCollector::SetUTSCQdotSource) and removal of a stray merge marker comment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/EnergyPlus/Photovoltaics.hh | Declares new helper to detect integrated PV modes. |
| src/EnergyPlus/Photovoltaics.cc | Defines integrated-PV detection helper; minor qualification cleanup in PV reporting. |
| src/EnergyPlus/HeatBalanceSurfaceManager.cc | Triggers final surface heat balance re-pass when integrated PV is present. |
Suppressed comments (1)
src/EnergyPlus/HeatBalanceSurfaceManager.cc:5223
- This change adds a new condition (
AnyCellIntegrationMode) that triggers an extra pass throughCalcHeatBalanceOutsideSurf/InsideSurfat the end of the HVAC timestep. There are existing unit tests forHeatBalanceSurfaceManagerbehavior, but none appear to cover this new PV-triggered re-simulation path; adding a targeted unit/integration test (e.g., asserting that a non-zeroQPVSysSourceapplied after the first pass affectsSurfQsrcHist/ surface temps on the final pass) would help prevent regressions of #11698.
Photovoltaics::HasBuildingIntegratedPV(state, AnyCellIntegrationMode);
if (LowTempRadSysOn || HighTempRadSysOn || HWBaseboardSysOn || SteamBaseboardSysOn || ElecBaseboardSysOn || CoolingPanelSysOn || SwimmingPoolOn ||
AnyCellIntegrationMode) {
// Solve the zone heat balance 'Detailed' solution
// Call the outside and inside surface heat balances
CalcHeatBalanceOutsideSurf(state);
CalcHeatBalanceInsideSurf(state);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
…urfOutsideTempHist.
|
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/EnergyPlus/HeatBalanceSurfaceManager.cc:249
- The new regression test only calls
UpdatePVIntegrationSourceand checks flags; it never exercises this resimulation routine or verifies that the updated sink changes surface temperatures/heat rates. Consequently, the ordering failure from #11698—and the possibility that a pending final pass is skipped—can still pass the tests. Add an integration-level test that runs the coupled surface/PV path and asserts the internal-source heat balance responds to PV extraction.
void ResimulateSurfaceHeatBalanceForPV(EnergyPlusData &state)
src/EnergyPlus/HVACManager.cc:1809
- A pending PV surface resimulation does not keep the HVAC iteration loop alive. This function is only called when
SimSelectedEquipmentis entered, butPVSurfaceHeatBalanceResimFlagis not part of the loop condition inSimHVAC. During the call, the final PV calculation can set this flag andSimElecCircuitsFlag; the subsequent electric-manager pass then unconditionally clearsSimElecCircuitsFlagon non-first iterations. For outside-face and exterior-cavity PV, no air/plant flag remains, so the loop exits with the newest sink unapplied and the resimulation is delayed until a later system timestep. Include the PV flag in the iteration gating or otherwise guarantee the pending surface pass runs before convergence is declared.
HeatBalanceSurfaceManager::ResimulateSurfaceHeatBalanceForPV(state);
|
|
|
|
|
|
joseph-robertson
marked this pull request as ready for review
August 21, 2026 17:50
|
|
|
|
|
|
mitchute
approved these changes
Sep 3, 2026
mitchute
left a comment
mitchute
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @joseph-robertson. I think this one is good, but we'll check regressions one more time before merging.
Comment on lines
+171
to
+177
| // Surface-coupled PV must be initialized before its first temperature-dependent calculation. | ||
| if (state.dataPhotovoltaicState->GetInputFlag && | ||
| state.dataInputProcessing->inputProcessor->getNumObjectsFound(state, "Generator:Photovoltaic") > 0) { | ||
| Photovoltaics::GetPVInput(state); | ||
| state.dataPhotovoltaicState->GetInputFlag = false; | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
I'm not proposing we change this now, just wondering why not integrate this with InitSuraceHeatBalance ?
Contributor
There was a problem hiding this comment.
That's a good point. It might fit better there, but maybe the thinking was it's only for PV not for "all surfaces" and so maybe it doesn't?
| } | ||
|
|
||
| for (int PVnum = 1; PVnum <= state.dataPhotovoltaic->NumPVs; ++PVnum) { | ||
| Photovoltaics::SimSurfaceCoupledPV(state, PVnum); |
Collaborator
There was a problem hiding this comment.
Hitting this before resolving the outside and inside heat balance. Seems fine.
Comment on lines
+251
to
+269
| void ResimulateSurfaceHeatBalanceForPV(EnergyPlusData &state) | ||
| { | ||
| // Repeat the coupled surface and PV calculations after electric simulation changes the PV heat sink. | ||
| if (!state.dataHVACGlobal->PVSurfaceHeatBalanceResimFlag) { | ||
| return; | ||
| } | ||
|
|
||
| for (int pass = 1; pass <= 2; ++pass) { | ||
| state.dataHVACGlobal->PVSurfaceHeatBalanceResimFlag = false; | ||
| CalcHeatBalanceOutsideSurf(state); | ||
| CalcHeatBalanceInsideSurf(state); | ||
| for (int PVnum = 1; PVnum <= state.dataPhotovoltaic->NumPVs; ++PVnum) { | ||
| Photovoltaics::SimSurfaceCoupledPV(state, PVnum); | ||
| } | ||
| if (!state.dataHVACGlobal->PVSurfaceHeatBalanceResimFlag) { | ||
| break; | ||
| } | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
New function to resim the outside and and inside surface heat balances, and then (?) resim the PV after. That's backwards from what you did earlier. Does it matter?
Contributor
There was a problem hiding this comment.
Isn't it true that by the time the simulation has hit this point that it has already gone through the inside and outside heat balance? So, why would it need to do another round of that (for all surfaces) if the point here is that something in the PV systems has to update and another round of the heat balance has to take place? In UpdateFinalSurfaceHeatBalance, another round is needed because those systems are running at the HVAC time step and the zone heat balance needs to update based on the "average" of what has been happening. Since PV is running at the HVAC time step level, is something similar happening here? Or is there a different reason that I'm not understanding yet? I guess, why not call SimSurfaceCoupledPV and then just use the UpdateFinalSurfaceHeatBalance with an updated flag?
| } | ||
|
|
||
| for (int pass = 1; pass <= 2; ++pass) { | ||
| state.dataHVACGlobal->PVSurfaceHeatBalanceResimFlag = false; |
Collaborator
There was a problem hiding this comment.
So, you set PVSurfaceHeatBalanceResimFlag to false, and then inside SimSurfaceCoupledPV it can drop into UpdatePVIntegrationSource, which then evaluates whether another iteration is needed. But only up to a max of 2. Seems OK, I think.
Contributor
There was a problem hiding this comment.
Is the pass max of 2 due to the impact on the outside surface heat balance and needing it to better update?
Collaborator
|
@RKStrand maybe you could take a quick look at this one, if you get a chance? |
Contributor
|
@mitchute Sure, I'll try to look it over tomorrow, hopefully in the morning. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request overview
ConstructionProperty:InternalHeatSourcenot being properly integrated with the surface heat balance #11698Description of the purpose of this PR
The issue was an ordering gap in the coupled PV thermal model. For building-integrated PV, electrical generation removes energy from the mounting surface, transpired collector, vented cavity, or PVT collector; previously that updated PV heat sink could be calculated after the associated surface heat balance had already been solved for the HVAC iteration. The fix publishes the changed sink to its owning thermal model and, when the change is material, requests a bounded surface/PV heat-balance resimulation plus any required air, plant, and electrical-loop passes. This reconciles the PV heat extraction with the local boundary temperatures and downstream collector response within the same iteration, covering the outside-face, transpired-collector, exterior-vented-cavity, and PVT integration modes.
IntegratedSurfaceOutsideFace: PV is recalculated from the updated surface outside-face temperature. A material PV heat-sink change requests another surface heat-balance pass so the wall surface and PV remain consistent in the HVAC iteration.
IntegratedExteriorVentedCavity: The current PV heat sink is republished to the exterior vented-cavity baffle model. A material change triggers surface/cavity/PV reconciliation so the cavity boundary condition and covered-surface temperature are refreshed.
IntegratedTranspiredCollector: The current PV heat sink is republished to the unglazed transpired collector energy balance. A material change requests surface heat-balance plus air- and plant-loop resimulation, reconciling collector temperature, plenum air, wall boundary conditions, and PV output.
PhotovoltaicThermalSolarCollector: The current PV heat sink is republished to the PVT collector. A material change requests air- and plant-loop resimulation so thermal-fluid conditions, thermal production, and PV output use a consistent collector state.
integrated-pv-four-modes.html
Pull Request Author
Reviewer