added overhang shading implementation for #1467 #1468
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jelgerjansen
requested changes
Oct 20, 2025
Contributor
jelgerjansen
left a comment
jelgerjansen
left a comment
There was a problem hiding this comment.
Thanks for implementing this @Mathadon. I've added some feedback and remaks.
I'm not very familiar with all calculations of angles, so for that I'm trusting your expertise on this.
| parameter Modelica.Units.SI.Length L(min=0) | ||
| "Distance to object perpendicular to window" | ||
| annotation (Dialog(group="Dimensions (see illustration in documentation)")); | ||
| parameter Modelica.Units.SI.Length LOv(min=0) = 0 |
Contributor
There was a problem hiding this comment.
I would unify the parameter naming of this model and the Overhang model. So either:
dep(as used in the Overhang model)LOve(use three characters) and apply this change to the Overhang model as well
| parameter Modelica.Units.SI.Length dh | ||
| "Height difference between top of object and top of window glazing" | ||
| annotation (Dialog(group="Dimensions (see illustration in documentation)")); | ||
| parameter Modelica.Units.SI.Length dhOv = 0 |
| parameter Modelica.Units.SI.Length LOv(min=0) = 0 | ||
| "Distance between overhang edge and wall, perpendicular to wall" | ||
| annotation (Dialog(group="Building shade", enable=hasBuildingShade)); | ||
| parameter Modelica.Units.SI.Length dhOv(min=-hWal) = 0 |
Contributor
There was a problem hiding this comment.
I would apply the min attribute in the BuildingShade model, in case someone uses this model for e.g. a window.
Comment on lines
+44
to
+49
| final parameter Real coeff = 1-fraSha | ||
| "More efficient implementation"; | ||
| final parameter Real coeffOv = 1-fraShaOv | ||
| "More efficient implementation"; | ||
| final parameter Real hWinInv = 1/hWin | ||
| "More efficient implementation"; |
Contributor
There was a problem hiding this comment.
- Update parameter declaration to something like "Intermediate parameter to simplify model equations and expressions"
- Do you really need the
coeffparameters? I guess the final expressions are more understandable iffraShaandfraShaOveare directly used.
| Modelica.Units.SI.Angle alt=(Modelica.Constants.pi/2) - angZen; | ||
| Modelica.Units.SI.Angle verAzi | ||
| Modelica.Units.SI.Length L2 = L1*tan(alt) | ||
| "Maximum height of object before it casts some shade"; |
Contributor
There was a problem hiding this comment.
"opposite object" (clarify now that the model has two different shading objects)
| final parameter Real fraSunDifSky(final min=0,final max=1, final unit="1") = 1-vieAngObj/(Modelica.Constants.pi/2) | ||
| "Fraction of the light that is absorbed, e.g. smaller than 1 for shading cast by tree lines."; | ||
| parameter Real fraShaOv(min=0,max=1) = fraSha | ||
| "Fraction of the light that is absorbed for overhang, e.g. smaller than 1 for shading cast by screens."; |
Contributor
There was a problem hiding this comment.
- absorbed by the overvang
- I find the example somewhat weird. Couldn't you say something like "equal to 1 for a concrete overhang and smaller than 1 for a glass overhang"
| parameter Real fraSha(min=0,max=1) = 1 | ||
| "Fraction of the light that is shaded, e.g. smaller than 1 for shading cast by tree lines."; | ||
| final parameter Real fraSunDifSky(final min=0,final max=1, final unit="1") = 1-vieAngObj/(Modelica.Constants.pi/2) | ||
| "Fraction of the light that is absorbed, e.g. smaller than 1 for shading cast by tree lines."; |
Contributor
There was a problem hiding this comment.
Fraction of the light that is absorbed by the opposite object
| extends IDEAS.Buildings.Components.Shading.Interfaces.PartialShadingDevice( | ||
| final controlled=false); | ||
|
|
||
| parameter Modelica.Units.SI.Length L(min=0) |
Contributor
There was a problem hiding this comment.
Update model documentation: "Component for modeling shade cast by distant opposite objects such as buildings and treelines and/or an overhang"
Comment on lines
37
to
+40
| parameter Modelica.Units.SI.Angle vieAngObj=atan((hWin/2 + dh)/L) | ||
| "Viewing angle of opposite object"; | ||
| parameter Modelica.Units.SI.Angle vieAngOv=if haveOverhang then Modelica.Constants.pi/2 - atan((hWin/2 + dhOv)/LOv) else 0 | ||
| "Viewing angle of overhang"; |
Contributor
There was a problem hiding this comment.
- Isn't view factor a better word?
- I would expect
vieAngOveto be equal toatan(LOve/(hWin/2+dhOve)). How did you derive your expression?
Comment on lines
+69
to
+76
| if noEvent(L2<dh) then | ||
| fraSunDir=coeff; | ||
| elseif noEvent(L2<dh+hWin) then | ||
| fraSunDir=coeff + (L2-dh)*fraSha*hWinInv; | ||
| elseif noEvent(L2Ov>dhOv+hWin) then | ||
| fraSunDir=coeffOv; | ||
| elseif noEvent(L2Ov>dhOv) then | ||
| fraSunDir=coeffOv + (L2Ov-dhOv)*fraShaOv*hWinInv; |
Contributor
There was a problem hiding this comment.
You first check L2 and then L2Ove, is it always guaranteed that only 1 elseif statement is true?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #1467