Feature/gsye 900 by hannesdiedrich · Pull Request #1912 · gridsingularity/gsy-e

hannesdiedrich · 2026-01-12T14:26:34Z

Reason for the proposed changes

Please describe what we want to achieve and why.

Proposed changes

INTEGRATION_TESTS_BRANCH=master
GSY_FRAMEWORK_BRANCH=master
SCM_ENGINE_BRANCH=master

… for this class

codecov · 2026-01-12T14:50:04Z

Codecov Report

❌ Patch coverage is 72.13115% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.09%. Comparing base (de39098) to head (4345160).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1912      +/-   ##
==========================================
- Coverage   71.12%   71.09%   -0.04%     
==========================================
  Files         149      149              
  Lines       14166    14300     +134     
  Branches     1879     1891      +12     
==========================================
+ Hits        10075    10166      +91     
- Misses       3555     3593      +38     
- Partials      536      541       +5

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nd state and only rely on GlobalConfig

spyrostz · 2026-01-13T09:21:04Z

src/gsy_e/gsy_e_core/sim_results/file_export_endpoints.py

+    """Return if area has a heat pump strategy without tanks."""
+    return isinstance(
+        area.strategy,
+        (HeatPumpStrategyWithoutTanks),


I think that brackets can be omitted here

Ah yes, will fix it.

spyrostz · 2026-01-13T09:31:06Z

src/gsy_e/models/strategy/energy_parameters/heatpump/heat_pump.py

+        target_temp_C_profile: Optional[Union[str, float, Dict]] = None,
+        target_temp_C_profile_uuid: Optional[str] = None,


Is the target temperature really needed here ? I would expect that the target temperature could be calculated and does not need to be an input here

Please see comment below.

spyrostz · 2026-01-13T09:37:36Z

src/gsy_e/models/strategy/energy_parameters/heatpump/heat_pump.py

+        self._update_last_time_slot_data(time_slot)
+
+        if not self._heat_demand_Q_J:
+            produced_heat_energy_kJ = self._calc_Q_kJ_from_energy_kWh(


Ok, now I see that the target temperature is only used to calculate the heat demand from electricity demand. In this case, I would prefer to use a constant value as a target temperature, rather than expect a profile, to uncomplicate the class constructor. This depends of course from the input data that we have from installations, however I somehow suspect that we will have trouble to find target temperature profiles. If we do have such data, then ignore this comment.

We do not have a target temperatures as profile , yet. Indeed I was thinking of a constant value for now but wanted to be future prove and enabled providing a profile. I thought that we prepare the profile beforehand with simply filling the constant value to all timeslots.
In the case of the chiller this has to be a profile anyway (we received a temperature profile of the underground water basin.
In general, both temperature profiles are used to derive the COP and the heat demand. Which is the main task of this new strategy. Without it, it is just a load. This is why I vote for keeping profiles as input.
What do you think?

Thanks for the explanation! I agree in general, however I am also thinking whether it makes sense to provide an option for a constant temperature, so that the user avoids this:

I thought that we prepare the profile beforehand with simply filling the constant value to all timeslots.

Thinking about it a second time though, it makes sense to implement it when we need it and not now. Therefore it is fine to leave as is.

Just FYI, thanks to our magnificent implementation of StrategyProfile, we do not have to construct a profile beforehand but can simply pass the constant value:

strategy=HeatPumpStrategyWithoutTanks( target_temp_C_profile=42, ...)

spyrostz · 2026-01-13T09:39:33Z

src/gsy_e/models/strategy/heat_pump.py



-class MultipleTankHeatPumpStrategy(TradingStrategyBase):
+class HeatPumpStrategyBase(TradingStrategyBase):


I assume that this class is only refactoring to a base class with no edits, right? If yes then I will not review it in detail.

spyrostz · 2026-01-13T09:44:48Z

src/gsy_e/models/strategy/state/heatpump_state.py

        return self.__class__.__name__
+
+
+class HeatPumpStateWithoutTanks(HeatPumpStateBase):


I think that this class is not needed, and the methods can be integrated in the base class (HeatPumpStateBase). That way, the caller code could instantiate the base class (or an alias name of the base class for readability) and avoid having to maintain this class.

Now that I look at it, I agree. Will do

spyrostz

Some comments, will LGTM once replied / fixed. Thanks!

…ithoutTanks.serialize

spyrostz · 2026-01-14T09:25:35Z

src/gsy_e/models/strategy/state/heatpump_state.py

+    def get_state(self) -> Dict:
+        return {
+            "cop": convert_pendulum_to_str_in_dict(self._cop),
+            "heat_demand_kJ": convert_pendulum_to_str_in_dict(self._heat_demand_kJ),
+        }
+
+    def restore_state(self, state_dict: Dict):
+        self._cop = convert_str_to_pendulum_in_dict(state_dict["cop"])
+        self._heat_demand_kJ = convert_str_to_pendulum_in_dict(state_dict["heat_demand_kJ"])
+
+    def get_results_dict(self, current_time_slot: DateTime) -> Dict:
+        retval = {
+            "cop": self.get_cop(current_time_slot),
+            "heat_demand_kJ": self.get_heat_demand_kJ(current_time_slot),
+        }
+        return retval


Sorry for the nit, however I think that it would be better to reuse these methods from the superclass. Essentially, instead of duplicating these lines here, IMO it would be better to call super().get_state() from the HeatPumpState.get_state() method. That way any new attributes that we might add in the base class in the future will end up in the dicts of the superclass.

spyrostz

One last comment, will LGTM once resolved.

…mp energy parameters

spyrostz

LGTM

hannesdiedrich added 4 commits January 8, 2026 14:48

GSYE-900: First implementation of heatpump without tanks strategy

681be53

GSYE-900: Fix issue in exports

9da8bcf

GSYE-900: Optimize HeatPumpEnergyParametersWithoutTanks and add tests…

59e28e9

… for this class

GSYE-900: Fix small typo

658fc76

hannesdiedrich requested a review from spyrostz January 12, 2026 14:26

GSYE-900: Remove self._slot_length from heat pump energy parameters a…

c4c6765

…nd state and only rely on GlobalConfig

spyrostz reviewed Jan 13, 2026

View reviewed changes

hannesdiedrich added 2 commits January 13, 2026 15:01

GSYE-900: Adapt to PR comments

fb77d05

GSYE-900: Add target temperature profile to HeatPumpEnergyParametersW…

4ae7e27

…ithoutTanks.serialize

spyrostz reviewed Jan 14, 2026

View reviewed changes

hannesdiedrich added 3 commits January 14, 2026 11:54

GSYE-900: Fix hen and egg problem with the initial COP in the heat pu…

64d4edd

…mp energy parameters

GSYE-900: Fix hen and egg problem with the initial COP in the heat pu…

6cd5343

…mp energy parameters

GSYE-900: Re-use super methods in HeatPumpState

4345160

spyrostz approved these changes Jan 14, 2026

View reviewed changes

hannesdiedrich merged commit 1a8de36 into master Jan 14, 2026
4 checks passed

hannesdiedrich deleted the feature/GSYE-900 branch January 14, 2026 17:24

hannesdiedrich restored the feature/GSYE-900 branch January 15, 2026 08:25

		target_temp_C_profile: Optional[Union[str, float, Dict]] = None,
		target_temp_C_profile_uuid: Optional[str] = None,



		class MultipleTankHeatPumpStrategy(TradingStrategyBase):
		class HeatPumpStrategyBase(TradingStrategyBase):

		return self.__class__.__name__


		class HeatPumpStateWithoutTanks(HeatPumpStateBase):

Comments

Conversation

hannesdiedrich commented Jan 12, 2026

Reason for the proposed changes

Proposed changes

Uh oh!

codecov bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spyrostz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spyrostz left a comment

Choose a reason for hiding this comment

Uh oh!

spyrostz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jan 12, 2026 •

edited

Loading