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

Annex60.Fluid.Interfaces.FourPortHeatMassExchanger is not used #353

Closed
mwetter opened this issue Nov 13, 2015 · 9 comments · Fixed by #639
Closed

Annex60.Fluid.Interfaces.FourPortHeatMassExchanger is not used #353

mwetter opened this issue Nov 13, 2015 · 9 comments · Fixed by #639
Assignees
Milestone

Comments

@mwetter
Copy link
Contributor

mwetter commented Nov 13, 2015

The model Annex60.Fluid.Interfaces.FourPortHeatMassExchanger is not used anywhere in the library, and hence it is also not part of any regression test.
I suggest to keep the model in the library as it is similar to TwoPortHeatMassExchanger and libraries the are based on Annex60 are using it, for example in Buildings.Fluid.Chillers.Carnot.

To avoid breaking the model, a regression test needs to be added.

@mwetter
Copy link
Contributor Author

mwetter commented Nov 23, 2015

If Carnot where to be moved to Annex60, it should be called Carnot_y.

All should comment whether this should be moved to the Annex60 library.

@thorade
Copy link
Member

thorade commented Nov 23, 2015

Is this related to #223 ??

@mwetter
Copy link
Contributor Author

mwetter commented Nov 23, 2015

#223 is much bigger as its main purpose is to model an actual heat pump, whereas the Carnot model is idealized and meant for applications where no data of the heat pump is available, e.g., it is solely based on thermodynamics 2nd law, plus an efficiency correction that optionally takes the load as an input.

@damienpicard
Copy link
Contributor

According to me, Annex60 could benefit of having the Buildings.Fluid.Chillers.Carnot model. I've revised the model and I've listed my comments in lbl-srg/modelica-buildings#471. Once these comments are adressed and if everybody agrees, the Carnot model can be added.

@damienpicard
Copy link
Contributor

As discussed in lbl-srg/modelica-buildings/issues/471, I did the following changes:
. Add a Carnot heat pump, and rename these models to Carnot_y (similarly than for the heater/cooler) to avoid a clash in names.
. remove actualStream in computing the states that are used to compute the Carnot efficiency.

I also added the example from the building library and i've updated the reference result.

I've pushed the changes on issue353_addCarnot_y

@mwetter
Copy link
Contributor Author

mwetter commented Jan 27, 2016

@damienpicard
I will also integrate the Chiller.Carnot_TEva, HeatPumps.Carnot_TCon and HeatPumps.Carnot_y.

I will also correct the COP computation so that the useful heat is the heat of the evaporator for the chiller, and the heat of the condenser for the heat pump.

To eliminate code duplication, I will refactor that Chiller.Carnot_y so that it uses a base class that can also be shared by HeatPump.Carnot_y, as is done for Chiller.Carnot_TEva and HeatPumps.Carnot_TCon.

@mwetter
Copy link
Contributor Author

mwetter commented Jan 30, 2016

The above models have been integrated and documented. The following still needs to be done:

  • Review the examples and validation cases, and make sure P, QCon_flow and QEva_flow are plotted.
  • Add validation cases in which the evaporator or condenser heat flow rate are limited.
  • Add validation cases in which the evaporator temperature is higher than the condenser temperature to see whether the model behaves reasonably if it gets into this domain.

mwetter added a commit that referenced this issue Feb 10, 2016
mwetter added a commit that referenced this issue Feb 10, 2016
mwetter added a commit that referenced this issue Feb 11, 2016
This is to comply with the coding convention. For #353
mwetter added a commit that referenced this issue Feb 17, 2016
For #400 and #353. Results changed slightly because of different default value for tau (as inherited from base class).
@damienpicard
Copy link
Contributor

@mwetter notice that Annex60.Fluid.Interfaces.FourPortHeatMassExchanger is still not used in the Annex60 library, as the implementation of the the Carnot model extends now PartialFourPortInterface instead of FourPortHeatMassExchanger.

@damienpicard damienpicard assigned mwetter and unassigned damienpicard Feb 19, 2016
@mwetter mwetter added this to the 1.0 release milestone May 10, 2016
@mwetter
Copy link
Contributor Author

mwetter commented Oct 23, 2016

Note from Corsica meeting: Michael to ensure that the model is used in a regression test in Annex60, and then we keep it in the library for release 1.0.

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 a pull request may close this issue.

3 participants