Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue1921 organic rankine cycle #1922

Merged
merged 19 commits into from
Oct 23, 2024
Merged

Issue1921 organic rankine cycle #1922

merged 19 commits into from
Oct 23, 2024

Conversation

hcasperfu
Copy link
Contributor

This closes #1921.

@FWuellhorst -
Greetings! Do you mind reviewing this PR?
A (hopefully) comprehensive documentation of this model is in the info tab of IBPSA.Fluid.CHPs.OrganicRankine.Cycle.
We are hoping to have this model clear IBPSA repo before we present it in the 2024 American Modelica Conference on 14-16 Oct.

@hcasperfu hcasperfu self-assigned this Aug 20, 2024
@FWuellhorst
Copy link
Contributor

@hcasperfu I should be able to provide a review in the next two weeks, if that fits with your schedule :)

@hcasperfu
Copy link
Contributor Author

@FWuellhorst -
That would be excellent! Thanks!

@hcasperfu hcasperfu requested a review from FWuellhorst August 21, 2024 16:26
Copy link
Contributor

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

Overall, the implementation and model looks good, with a thourugh documentation and nice plots.
Aside from my in-line comments, I have some questions / concerns:

  1. How can users provide a constant value for evaporation temperature? Is the use-case always a constant temperature waste-heat? At least for Carnot-Batteries, the inlet temperature will decrease over time and thus the TEva would need to decrease, as well. Compared to heat pumps, I would expect the internal control to adjust the evaporating pressure / temperature in such a way that the expander outlet is outside "the dome". Or does it all just work based on the amount of mass flow rate in ORC? In the end, it would be nice to see some comparison to data provided in a ORC datasheet or similar to see if nominal evaporating temperatures, mass flow rates, and pinch points are typically known or not.
  2. As no actual validation is given, I can not review if the model itself makes sense or not. The equations and physics look good, but whether the constant pinch points and evaporating temperature is realistic is hard to say.
  3. Wouldn't have expected anything different, but most naming and docs look good. The script we have in AixLib to adhere to naming guidelines found the following possibly relevant violations. The specific names (q, w) and endings (reg, T, set, actual) are not listed in the Buildings guideline as far as I know.
IBPSA\Fluid\CHPs\OrganicRankine\Examples\ORCHotWater.mo
3: Name 'TWatOut_set' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Out_set. Affected line: Modelica.Blocks.Sources.Constant TWatOut_set(k=55 + 273.15) "Set point of hot water output" annotation (Placement(transformation(extent={{0,0},{20,20}})));

4: Missing documentation. Affected line: Modelica.Blocks.Logical.And and1 annotation (Placement(transformation(extent={{-140,40},{-120,60}})));

5: Missing documentation. Affected line: IBPSA.Fluid.Sensors.TemperatureTwoPort senTColOut( redeclare final package Medium = MediumCol, m_flow_nominal=mCol_flow_nominal, T_start=TCol_start) annotation (Placement( transformation( extent={{10,-10},{-10,10}}, rotation=0, origin={-70,-40})));



IBPSA\Fluid\CHPs\OrganicRankine\BaseClasses\InterpolateStates.mo
1: Name 'qCon' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: q. Affected line: Modelica.Units.SI.SpecificEnergy qCon = hExpOut - hPumInl "Condenser specific energy transfer out of the cycle";

2: Name 'wExp' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: w. Affected line: Modelica.Units.SI.SpecificEnergy wExp = hExpOut - hExpInl "Expander specific work";

3: Name 'wPum' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: w. Affected line: Modelica.Units.SI.SpecificEnergy wPum = (pEva - pCon) / (rhoLiq * etaPum) "Pump specific work";

4: Name 'h_reg' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: h_reg. Affected line: protected Modelica.Units.SI.SpecificEnthalpy h_reg = hExpOutDry - hSatVapCon "For regularisation; if > 0, dry cycle";

5: Name 'rhoLiqDer_T' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Der_. Affected line: final parameter Real rhoLiqDer_T[pro.n]= IBPSA.Utilities.Math.Functions.splineDerivatives( x = pro.T, y = pro.rhoLiq, ensureMonotonicity = true) "Derivative of saturated liquid density vs. temperature for cubic spline";

6: Name 'sSatLiqDer_T' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Der_. Affected line: final parameter Real sSatLiqDer_T[pro.n]= IBPSA.Utilities.Math.Functions.splineDerivatives( x = pro.T, y = pro.sSatLiq, ensureMonotonicity = true) "Derivative of saturated liquid entropy vs. temperature for cubic spline";

7: Name 'sSatVapDer_T' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Der_. Affected line: final parameter Real sSatVapDer_T[pro.n]= IBPSA.Utilities.Math.Functions.splineDerivatives( x = pro.T, y = pro.sSatVap, ensureMonotonicity = false) "Derivative of saturated vapor entropy vs. temperature for cubic spline";

8: Name 'hSatLiqDer_T' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Der_. Affected line: final parameter Real hSatLiqDer_T[pro.n]= IBPSA.Utilities.Math.Functions.splineDerivatives( x = pro.T, y = pro.hSatLiq, ensureMonotonicity = true) "Derivative of saturated liquid enthalpy vs. temperature for cubic spline";

9: Name 'hSatVapDer_T' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Der_. Affected line: final parameter Real hSatVapDer_T[pro.n]= IBPSA.Utilities.Math.Functions.splineDerivatives( x = pro.T, y = pro.hSatVap, ensureMonotonicity = false) "Derivative of saturated vapor enthalpy vs. temperature for cubic spline";

IBPSA\Fluid\CHPs\OrganicRankine\Cycle.mo
1: Name 'dTPinEva_set' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Eva_set. Affected line: parameter Modelica.Units.SI.TemperatureDifference dTPinEva_set( final min = 0) = 5 "Set evaporator pinch point temperature difference" annotation(Dialog(group="Evaporator"));

6: Name 'mWor_flow_hysteresis' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Wor_flow_hysteresis. Affected line: parameter Modelica.Units.SI.MassFlowRate mWor_flow_hysteresis = mWor_flow_min + (mWor_flow_max - mWor_flow_min) * 0.1 "Hysteresis for turning off the cycle when flow too low" annotation(Dialog(group="Cycle"));

7: Name 'on_actual' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: on_actual. Affected line: Modelica.Blocks.Interfaces.BooleanOutput on_actual "Actual on off status of the cycle" annotation (Placement(transformation( extent={{100,-20},{140,20}}), iconTransformation(extent={{100,-10},{ 120,10}})));

IBPSA\Fluid\CHPs\OrganicRankine\BaseClasses\FixedEvaporating.mo
1: Name 'dTPinEva_set' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Eva_set. Affected line: parameter Modelica.Units.SI.TemperatureDifference dTPinEva_set( final min = 0) "Set evaporator pinch point temperature difference" annotation(Dialog(group="Evaporator"));

5: Name 'mWor_flow_hysteresis' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: Wor_flow_hysteresis. Affected line: parameter Modelica.Units.SI.MassFlowRate mWor_flow_hysteresis( final min = 0) "Hysteresis for turning off the cycle when flow too low" annotation(Dialog(group="Cycle"));

6: Name 'on_actual' contains parts with more/less than 3 characters or which are not part of special cases. Affected parts: on_actual. Affected line: Modelica.Blocks.Interfaces.BooleanOutput on_actual = ena and hys.y "Actual on off status of the cycle" annotation (Placement(transformation( extent={{100,-20},{140,20}}), iconTransformation(extent={{100,-10},{120, 10}})));

IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/package.order Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
IBPSA/Fluid/CHPs/OrganicRankine/Cycle.mo Outdated Show resolved Hide resolved
@hcasperfu
Copy link
Contributor Author

@FWuellhorst -
Thank you very much for the extensive review and terribly sorry for taking me so long to get back to this.

  1. Much of the system and control assumptions of this model were based on Quoilin et al. 2011. It stated that "The most common control strategy is to define a constant evaporating temperature and superheating", although the evaporating temperature $T_{eva}$ needs to be determined by prior analysis. It then based the control strategy on the working fluid mass flow rate $\dot m_{wor}$ because it was the most "easily accessible measurement". The cycle reacts to a variable heat source by adjusting $\dot m_{wor}$ to maintain the pinch point at a fixed $T_{eva}$.
  2. The validation model was primarily designed to test whether the model breaks at zero flow rates, as is how they are intended in the Buildings library. We do not have relevant data readily available for experiment-based tests.
  3. Indeed the specific terms, interestingly, are not listed in the naming convention. But examples using similar names can be found in the Buildings library.

I will work on your inline comments shortly.

@FWuellhorst
Copy link
Contributor

@hcasperfu
Sure, glad to provide feedback.
Regarding your other points:

  1. Ok, but this paper is not mentioned in the doc. Maybe I would state it in the general information "... as a bottoming cycle based on XY et al.". Based on your clarifications, this model is only suited for waster-water applications, or? We have a project for Carnot-Batteries. In this case, the constant evaporation temperature is of no good and the model would fail to work. This is why I feel strongly about a fitting name instead of just ORC.
  2. To be discussed in working group.
  3. Ok, maybe you have to update the naming convention wiki then.

Copy link
Contributor

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

@hcasperfu All changes look good, only the images have to be renamed, as well. I don't think adding the script directly is necessary, you could do that upon adding the reference to the conference paper in my opinion. I will be on vacation in the upcoming weeks and not able to further review this PR if you do further changes.

@hcasperfu hcasperfu requested a review from mwetter October 11, 2024 19:34
@hcasperfu
Copy link
Contributor Author

@mwetter -
This PR has cleared external review and ready to merge.

Copy link
Contributor

@mwetter mwetter left a comment

Choose a reason for hiding this comment

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

This result is wrong:
image
Produced from
simulateModel("IBPSA.Fluid.CHPs.OrganicRankine.Examples.ORCHotWater", method="Cvode", stopTime=900, tolerance=1e-06, resultFile="ORCHotWater");
Also, generating the html documentation gives
Can not resolve link "Modelica://IBPSA.DHC.ETS.Combined.Subsystems.Validation.Chiller" in [IBPSA.Fluid.CHPs.OrganicRankine.Examples.ORCHotWater](modelica://IBPSA.Fluid.CHPs.OrganicRankine.Examples.ORCHotWater)

In Dymola, IBPSA.Fluid.CHPs.OrganicRankine.Examples.ORCHotWater gives warning due to non-initialized iteration variables. Set a start value to avoid this.

@hcasperfu
Copy link
Contributor Author

@mwetter -

Thanks for the review!

  1. The incorrect variable is now deleted in 40b0f48. I messed up the sign in the equation which also had a guard against zero. Because this top-level variable was not used anywhere and the same variable is available in the component, I simply deleted it. The same variable at a lower level was correct.
image
  1. I removed the unresolved link in 2805b60. It referred to a model only available in Buildings and it did was say "this is the same as that".
  2. I added start values to suppress those Dymola translation warnings in 073ad76.

@mwetter mwetter merged commit fde0d12 into master Oct 23, 2024
3 checks passed
@mwetter mwetter deleted the issue1921_organicRankineCycle branch October 23, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Organic Rankine Cycle
3 participants