Boiler Fails with High Outside Air Temp

[s2t2]

This assertion in the boiler causes simulations to fail on hot days (like Oct 6 2024) where the outdoor air temp exceeds the hot water temp.

sbsim/smart_control/simulator/boiler.py

Line 296 in c230691

assert water_temp >= outside_temp

Besides changing this non-test assertion into a value error (which we already have an issue about #28 ), it would be very helpful to learn more about why this assertion is in place, and if it is necessary. Ideally we would like to implement a fix that allows the simulation to work on hot days.

@Jgoldfeder do you have any context to share about this logic?

[s2t2]

Copilot assessment:

---

I inspected the file you pointed to (smart_control/simulator/boiler.py) and found the assertion you mentioned:

• Location: compute_thermal_dissipation_rate (around line 296 in the version you gave).
• Current assertion: assert water_temp >= outside_temp

What the assertion does and why it was likely added

• The assertion enforces water_temp >= outside_temp so that delta_temp = water_temp - outside_temp is non‑negative.
• With the formula used, when delta_temp is positive the function returns a positive thermal dissipation rate (Watts lost from tank to environment). The author probably used the assertion to ensure the function only models heat loss (dissipation) and never heat gain from the environment.
• It prevents the calling code from seeing negative values (which would mean net heat gain from the environment into the tank).

Why this breaks when outside temperatures are high

• If outside_temp > water_temp (e.g., very warm outdoor air), the assertion fails and raises AssertionError, causing the simulation to crash in warm conditions.
• In many climates or situations (solar heating of plant room, heat island, etc.), outside_temp can exceed water_temp—this is physically plausible, and a simulator should not crash for that case.

Is the assertion necessary?

• No, the assertion is not strictly necessary from a physics standpoint. In fact, physics predicts that if outside_temp > water_temp there will be a net heat flow into the tank (i.e., dissipation < 0), which is meaningful and should be representable by the model.
• The assertion is a design choice to forbid representing ambient heating (it assumes "dissipation" should be non-negative). But it is overly restrictive if you want to handle warm outdoor temps or model passive heating of the tank.

Alternatives and recommended approaches
Below are three reasonable options, with tradeoffs and a recommended choice.

1. Remove the assertion and document that the function can return negative values

• Behavior: return a negative thermal rate when outside_temp > water_temp (meaning net heat gain).
• Pros: physically correct and simple; the rest of the simulation can reduce required boiler heating automatically.
• Cons: callers must correctly interpret negative dissipation (they should add it into energy balance as a negative term).
• Recommended when: you want physically consistent simulation and the rest of the code already handles negative thermal terms or can be updated to do so.

2. Clamp to zero (never return negative)

• Behavior: if outside_temp > water_temp, treat dissipation as zero (no heat loss; no heat gain).
• Pros: no negative rates to worry about; safer if callers assume dissipation >= 0.
• Cons: physically incorrect when ambient heating is real; you lose the benefit of ambient pre‑heating.
• Recommended when: existing code depends on dissipation being non‑negative and you cannot change other modules.

3. Make it configurable (allow_ambient_heating boolean)

• Behavior: introduce a class attribute (e.g., self._allow_ambient_heating) or function argument to choose between (1) and (2).
• Pros: flexible, backward compatible if defaulted to previous behavior.
• Cons: slightly more code and a minor API change.

Suggested minimal change (remove the assertion and document behavior)
This is the simplest fix and makes the simulator robust when outside temps are high.

Proposed function replacement (only the function shown — this change removes the assertion and documents negative return meaning):

def compute_thermal_dissipation_rate(
    self, water_temp: float, outside_temp: float
) -> float:
  """Returns the amount of thermal loss in W from a boiler tank.

  Thermal dissipation is the loss of heat due from the tank to the environment
  due to imperfect insulation, measured in Watts.

  The tank is assumed to be a cylindrical annulus, with an internal radius
  internal length, and an insulation thickness. Heat is dissapated only
  through the cylinder walls, and no heat is lost through the ends/caps.

  The equation is computed by applying an energy balance of:
  Q = Q_conduction = Q_convection, where Q_condition is the heat transferred
  from water at water_temp (T1) to the exterior surface temp (T2), and
  Q_convection is the transfer of the heat from the external surface at T2
  to the outside at T_inf.

  Q_conduction = (T1 - T2) / (ln(R2/R1) / 2 x pi x L x k), and
  Q_convection = (T2 - Tinf) / (1 / h x A), where
    A = the area of the annulus = 2 x pi x R2 x L

  Solving for Q:
  Q = (T1 - Tinf) / [ln(R2/R1) / 2pi / L / k + 1 / h / A]

  Note: This function returns positive values when the tank is losing heat to
  the environment (water_temp > outside_temp). When outside_temp > water_temp
  the returned value will be negative, indicating a net heat gain from the
  environment into the tank (ambient heating).

  Source: Heat Transfer, E. M. Sparrow, 1993

  Args:
    water_temp: average temperature of the water [K]
    outside_temp: temperature outside of the tank, can be ambient [K]

  Returns:
    thermal loss rate of the tank in Watts (may be negative if the tank gains
    heat from the environment)
  """

  delta_temp = water_temp - outside_temp
  numerator = self._tank_length * 2.0 * np.pi * delta_temp
  interior_radius = self._tank_radius
  exterior_radius = interior_radius + self._insulation_thickness
  conduction_factor = (
      np.log(exterior_radius / interior_radius)
      / self._insulation_conductivity
  )
  convection_factor = 1.0 / self._convection_coefficient / exterior_radius
  return numerator / (conduction_factor + convection_factor)

If you prefer the clamp-to-zero approach, replace the return line with:

raw_rate = numerator / (conduction_factor + convection_factor)
return max(0.0, raw_rate)

If you want configurability, add an attribute in init (e.g., allow_ambient_heating=True) and use:

raw_rate = numerator / (conduction_factor + convection_factor)
if not self._allow_ambient_heating:
  return max(0.0, raw_rate)
return raw_rate

Other considerations and testing

• Verify compute_thermal_energy_rate and any other callers correctly handle negative dissipation. compute_thermal_energy_rate currently sums dissipation_energy_rate with flow and tank heating terms. Allowing negative dissipation reduces the boiler heat demand automatically; that is physically consistent.
• Add unit tests:
  • Case outside_temp < water_temp -> dissipation > 0 (heat loss)
  • Case outside_temp == water_temp -> dissipation == 0
  • Case outside_temp > water_temp -> dissipation < 0 (heat gain) OR dissipation == 0 if you choose clamp behavior
• Check any logging, plotting, meter aggregation, or energy accounting that may assume dissipation >= 0 and update them to accept negative contributions (or clamp at aggregation time if that’s the intended behavior).

What I did and next steps

• I inspected the function, explained why the assertion exists, why it's overly restrictive, and suggested three alternatives (remove assertion, clamp to zero, or make behavior configurable).
• Next, you can pick the policy you want (allow ambient heating or disallow it). If you want, I can:
  • open a small PR with the minimal change (remove assertion + updated docstring),
  • or open a PR that adds a class-level option (allow_ambient_heating) with tests,
  • or provide the exact patch/diff you can apply locally.

Which option would you like me to implement?

[Mustaphayinka]

I'd like to implement this, my inclination is Option 1 which is remove the assertion, allow negative dissipation, document that it represents ambient heat gain, since it directly fixes the crash and matches the physics. Happy to add the allow_ambient_heating config option instead if you'd prefer that for backward compatibility. Let me know which direction to go and I'll open a PR.

[s2t2]

Thanks @Mustaphayinka .

We have a sync-out coming soon from our internal Google codebase. We might be inclined to wait until that happens before digging deep on this.

However in the meantime, I wonder if @amcberkes has any thoughts or context to share about this issue, as she is more knowledgeable about the simulator logic side of things.

[Mustaphayinka]

Thanks for the feedback! I'll verify the links resolve locally and fix the line-length issues with scoped pylint disables around the docstrings.

[amcberkes]

@s2t2 I haven't done any work on the boiler (only the ASHP). Option 1 sounds good to me but we should double check with John probably.