-
Notifications
You must be signed in to change notification settings - Fork 82
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
initial implementation of BoilerGeneric and Example #1443
base: main
Are you sure you want to change the base?
Conversation
I looked roughly over the code and saw some issues regarding the naming convention. I am mentioning them here so that you can consider them in your further development. You use "Nom" as an abbreviation in some models (e.g., in |
Thank you for making a Pull Request to AixLib! Our CI pipeline will help you finalize your contribution.
If HTML errors occur, I will fix the issues using a separate pull request. Tips to fix possible naming violations:
If all CI stages pass and you have addressed possible naming violations, please consider the following:
Once you have addressed these points, you can assign a reviewer. If you have any questions or issues, please tag a library developer. |
…ion reference files. Please pull the new files before push again. Plottet Results /issue1147_GenericBoiler/charts/
…ion reference files. Please pull the new files before push again. Plottet Results /issue1147_GenericBoiler/charts/
…/AixLib into issue1147_GenericBoiler
…ion reference files. Please pull the new files before push again. Plottet Results /issue1147_GenericBoiler/charts/
…ion reference files. Please pull the new files before push again. Plottet Results /issue1147_GenericBoiler/charts/
|
…/AixLib into issue1147_GenericBoiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Find my comments below based on the git diff, I will look into the models itself once these points are addressed 👍
AixLib/DataBase/Boiler/General/Boiler_Generic_Performance_Map.sdf
Outdated
Show resolved
Hide resolved
|
||
|
||
|
||
Modelica.Blocks.Sources.RealExpression ReturnTemp(y=TRet_nominal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retTem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wont check further naming convention, please adhere to the errors in the .txt
@@ -0,0 +1,163 @@ | |||
within AixLib.Fluid.BoilerCHP.BaseClasses; | |||
model DesignOperation "Operating for design conditions" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the doc
<p><span style=\"font-family: Arial;\">The nominal adiabatic efficiency (reference is the higher heating value (includes vaporization enthalpy)) comes from the SDF and depends on:</span></p> | ||
<ul> | ||
<li><span style=\"font-family: Arial;\">Nominal return temperature (TColdNom)</span></li> | ||
<li><span style=\"font-family: Arial;\">Nominale temperature difference (TColdNom-THotNom)</span></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a parameter if it is only the nominal value?
Also, check Arial font
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output is only based on nominal conditions but exists for multiple setups, because TSup_nominal
and TRet_nominal
are input parameters. As the data is behind the calculation is pre calculated anway, we can also put this data into the .sdf file. Removed the font issues with commit 07db990
"Design return temperature" annotation (Dialog(group="Design"),Evaluate=false); | ||
|
||
|
||
package Medium=AixLib.Media.Water; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaceable as usual?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adressed with 07db990
AixLib/Resources/ReferenceResults/Dymola/AixLib_Fluid_BoilerCHP_Examples_CHPSystem.txt
Outdated
Show resolved
Hide resolved
AixLib/Systems/ModularEnergySystems/ModularBoiler/Controls/heatingCurve.mo
Outdated
Show resolved
Hide resolved
parameter Integer k=2 "number of consumers"; | ||
package MediumWater = AixLib.Media.Water; | ||
|
||
ModularBoiler modularBoiler( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full paths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed with 479ad5a
parameter Boolean hasPump=true "Model includes a pump" | ||
annotation (choices(checkBox=true), Dialog(descriptionLabel=true, group="System setup")); | ||
parameter Modelica.Units.SI.HeatFlowRate Q_flow_nominal=50000 | ||
"Thermal dimension power" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nominal heat flow rate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, much better, fixed with 8fb8bc0
…re is no information where they are coming from #1443
This reverts commit 479ad5a.
# Conflicts: # AixLib/Fluid/BoilerCHP/BaseClasses/PartialHeatGenerator.mo # AixLib/package.mo
b4c8a03
to
70b9fe0
Compare
This includes the first model of the new ModularEnergySystems models (see #1147)
Before merge we want to solve all todos listed in #1439