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

SimInfoManager - Azimuth vs. inclination #1313

Open
StijnVandePutte opened this issue Jan 13, 2023 · 2 comments
Open

SimInfoManager - Azimuth vs. inclination #1313

StijnVandePutte opened this issue Jan 13, 2023 · 2 comments

Comments

@StijnVandePutte
Copy link

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
Copy link
Contributor

@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
Copy link
Member

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.

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

No branches or pull requests

3 participants