Update units and parameter propagation #42
Conversation
benjamsy
commented
Feb 21, 2023
•
benjamsy
commented
- Change units for library from unspecified type to Modelica SI/NonSI units
- Fixed issue for example models where some defined parameters was not translated to correct levels
- Added parameter propagation for number of cells in stack to subsystem control
- Added lower and upper limits to parameter list for EMS block
- Added parameter grouping for rangeExtenderPowertrain
-Change units for library from unspecified type to Modelica SI/NonSI units -Fixed issue for example models where some defined parameters was not translated to correct levels -Added parameter propagation for number of cells in stack to subsystem control -Added lower and upper limits to parameter list for EMS block -Added parameter grouping for rangeExtenderPowertrain
kvid
changed the title
-Removed long units paths -Changed declaration of constants
There was a problem hiding this comment.
I like this great work you've done using standardized type declarations, and are trying something similar for XInTheLoop. However, when browsing through your changes to learn best practice, I also found a few issues where I wonder if you have considered alternatives. See my comments connected to those code locations. Some of these issues are present at several code locations with some variations, so if you decide to change any of these, please also search for the other locations with similar code, because I didn't want to repeat the same question at all locations.
| // Thermal parameters | ||
| parameter Real heatCapacity(unit = "J/(kg.K)") = 800 "Specific Heat Capacity"; | ||
| parameter SI.SpecificHeatCapacity heatCapacity = 800 "Specific Heat Capacity"; | ||
| // Stack design parameters | ||
| parameter Real N_cell(unit = "1") = 1 "Number of Cells"; |
There was a problem hiding this comment.
Have you considered using Integer instead of Real for this parameter? There are also several other model files containing similar count parameters, currently declared as Real. I wonder why there is no standard declaration like e.g. type Count = Integer(unit = "1"); - is this not recommended in Modelica?
| parameter Real SOC_lower_limit(unit = "1") = 0.2 "SOC lower limit"; | ||
| parameter Real SOC_higher_limit(unit = "1") = 0.8 "SOC lower limit"; |
There was a problem hiding this comment.
Have you considered declaring e.g. type StateOfChargeRatio = Modelica.Units.SI.DimensionlessRatio(min = 0, max = 1) as type for all SOC parameters (also in other model files)?
| parameter Real rho_air(unit = "kg/m3") = 1.2 "Volumic mass of the air"; | ||
| parameter Real A_front(unit = "m2") = 2.7 "Front area of the vehicle"; | ||
| parameter SI.Density rho_air = 1.2 "Volumic mass of the air"; | ||
| parameter SI.Area A_front = 2.7 "Front area of the vehicle"; | ||
| parameter Real C_D(unit = "1") = 0.26 "Drag coefficient"; |
There was a problem hiding this comment.
Have you considered using Modelica.Units.SI.DimensionlessRatio (or can Modelica.Units.SI.Efficiency be a candidate)?
| parameter Real C_D(unit = "1") = 0.26 "Drag coefficient"; | ||
| parameter Real D_tire(unit = "m") = 0.4318 "Tire Diameter"; | ||
| parameter SI.Diameter D_tire = 0.4318 "Tire Diameter"; | ||
| parameter Real R_gear(unit = "1") = 3.478 "Reduction Gear Ratio"; |
There was a problem hiding this comment.
Have you considered using Modelica.Units.SI.DimensionlessRatio?
| parameter SI.Voltage V_rated_FC_stack = 57.9 "FC stack maximum operating voltage"; | ||
| parameter SI.Current I_rated_FC_stack = 300 "FC stack minimum operating voltage"; | ||
| parameter SI.Current i_L_FC_stack = 1.7 * I_rated_FC_stack "FC stack maximum limiting current"; | ||
| parameter SI.Current I_nom_FC_stack = 0.25 * I_rated_FC_stack "FC stack maximum limiting current"; | ||
| parameter Real N_FC_stack(unit = "1") = floor(V_rated_FC_stack / 0.6433) "FC stack number of cells"; |
There was a problem hiding this comment.
Have you considered declaring a constant representing the value 0.6433 that seems to be used several places? Maybe something like final constant SI.Voltage V_FC_something = 0.6433, but I don't know what should be the correct name of such a constant.
| SI.Pressure p_H2(min = 0); | ||
| SI.Pressure p_O2(min = 0); | ||
| SI.Pressure p_0 = 100000; |
There was a problem hiding this comment.
I suggest replacing SI.Pressure with SI.AbsolutePressure all places where absolute pressure is actually used (min=0 will then be declared in the type and hence doesn't need to be declared for each variable), or with SI.PressureDifference any other places (assuming atmospheric pressure as reference pressure unless otherwise noted).
I also suggest declaring the constant value as
final constant SI.AbsolutePressure p_0 = 100000 "Approximate atmospheric pressure";
