Closed bilwint closed 2 months ago
Hey @bilwint,
thank you for your work. I think, you found an elegant way to generalize the components offdesign parametrization. In my opinion, your PR looks mostly great. Some smaller remarks before we can merge:
HeatPumpEconIHX
class still implements the offdesign_simulation
method. Is this necessary for some reason or was it an oversight?'A'
-cycle, it is not strictly necessary to also save the design massflow of the 'D'
-cycle in multicyclic heat pumps. On the other hand, it is no problem to save them anyways.HeatPumpEcon
, HeatPumpSimpleTrans
and HeatPumpSimple
classes reimplement the intermediate_states_offdesign
method only for it to pass
, which is not needed, as it is already inherited that wayget_pressure_levels
methods of transcritical heat pumps had to be adjusted for them to work with offdesign simulation?As far as I can tell, if you could refer to the points above and clear those up, there is nothing else in the way towards merging your PR.
Best regards.
Hey @jfreissmann
offdesign_simulation
for the case of HeatPumpEconIHX
, is just a mistakeI agree with your second point. The advantage of the previous implementation was, that every heat pump class has the same attribute, that contains the design mass flow in the 'A'
-cycle and only that. But this uniformity is broken with things like self.wf
vs. self.wf1
+ self.wf2
etc. anyway, so maybe it is non really needed.
Regarding the changes of some of the get_pressure_levels
method: we use it mostly to get good starting values for the initial design simulation or for calculating the correct intermediate pressure. The former usage sould not affect the offdesign simulation, but for the latter case it makes sense, that changing the source temperature from outside the get_pressure_levels
method should result in a differrent intermediate pressure. I am not 100% sure, if we correctly do that right now in all models depending on an intermediate pressure though.
Sorry I misunderstood the fourth question regarding the get_pressure_levels
for trans models.
The get_pressure_levels
function had to adjust to implement generalized off-design. Because in the previous case there was no arguments in get_pressure_levels(self, wf=None)
. So it was not convenient to find intermediate states (p/T). That's why I implemented arguments to the get_pressure_levels(self, T_evap, wf=None)
of trans models.
Is that was your question?
Yes, that does answer my question. When you adjust the other open points from my initial comment, we can merge the PR.
11
Todo