search

open-ideas / IDEAS

Modelica library allowing simultaneous transient simulation of thermal and electrical systems at both building and feeder level.
131 stars 52 forks source link

SimInfoManager - Azimuth vs. inclination #1313

Open StijnVandePutte opened 1 year ago

StijnVandePutte commented 1 year ago

Small remark: In the model 'Boundary conditions > Interfaces > PartialSimInfoManager', the parameter names and descriptions for the orientations refer to an inclination instead of an azimuth.

partial model PartialSimInfoManager ... parameter Modelica.Units.SI.Angle incS=IDEAS.Types.Azimuth.S "South inclination"; parameter Modelica.Units.SI.Angle incW=incS + Modelica.Constants.pi/2 "West inclination"; parameter Modelica.Units.SI.Angle incN=incS + Modelica.Constants.pi "North inclination"; parameter Modelica.Units.SI.Angle incE=incS + 3*Modelica.Constants.pi/2 "East inclination";

parameter Modelica.Units.SI.Angle incAndAziInBus[:,:]={{IDEAS.Types.Tilt.Ceiling, 0},{IDEAS.Types.Tilt.Wall,incS},{IDEAS.Types.Tilt.Wall,incW},{IDEAS.Types.Tilt.Wall, incN},{IDEAS.Types.Tilt.Wall,incE},{IDEAS.Types.Tilt.Floor,0}} "Combination of inclination and azimuth which are pre-computed and added to solBus." annotation (Dialog(tab="Incidence angles")); final parameter Modelica.Units.SI.Angle aziOpts[5]={incS,incW,incN,incE,incS} "Inclination options, default south"; final parameter Modelica.Units.SI.Angle incOpts[4]={IDEAS.Types.Tilt.Wall, IDEAS.Types.Tilt.Floor,IDEAS.Types.Tilt.Ceiling,IDEAS.Types.Tilt.Wall} "Azimuth options, default wall";

jelgerjansen commented 1 year ago

@Mathadon how do you like me to tackle this issue? The documentation can be changed easily, but changing the parameter names from incS to aziS might lead to problems in other models? However, at first sight it seems that the parameters are only used in the PartialSimInfoManager model, so I would create a new branch and check the unit tests after updating.

Mathadon commented 1 year ago

I can only make suggestions =)

It's worth updating this because it is indeed a bit misleading but it would break pretty much all models. My suggestion would be to park this for later and to include it once a major IDEAS update (version 4.0.0) is made.