-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Addresses #3216, wrong OS:Daylighting:Control rotation angle translated #3858
Conversation
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.
Based on discussion on #3216 with screenshots and former comment from Dan, it seems that the fix is correct.
} | ||
|
||
// glare | ||
double glareAngle = -openstudio::radToDeg(primaryDaylightingControl->thetaRotationAroundYAxis()); | ||
double glareAngle = primaryDaylightingControl->phiRotationAroundZAxis(); |
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 radToDeg shouldn't be there, great catch. Everything stored as deg. OpenStudio uses euler angles internally only
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 think the "-" was needed actually. @eringold has a file that is throwing in 3.2.1 but worked as 2.9.0, and I tracked it down to here.
** Severe ** <root>[Daylighting:Controls][TZ_L6_BKS_Corridor/Toilets DaylightingControls][glare_calculation_azimuth_angle_of_view_direction_clockwise_from_zone_y_axis] - "-0.000000" - Expected number greater than or equal to 0.000000
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.
Agree that the negative sign should stay. If E+ requires rotation angle to be positive you could also do a:
if (angle < 0){
angle += 360;
}
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.
By "also" you mean "in addition" right?
If I follow correctly, openstudio's convention for phi is counterclockwise, which is the opposite of the convention for phi in E+ (clockwise)?
do you recall if it's documented somewhere (at least in code comments somewhere)?
Only thing I could find is this:
// rotate negative amount around the z axis, EnergyPlus defines rotation opposite to OpenStudio |
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.
@jmarrec yes I meant in addition, if after applying the negative sign the result for E+ is still negative you can add 360. The angle mod 360 might also work, you could try it with a unit test. If you use the right hand rule, https://en.wikipedia.org/wiki/Right-hand_rule, with y pointing North and x pointing East, then a positive rotation around the z-axis (pointing up) is counter-clockwise when viewed from above. The E+ convention of positive rotation in the clockwise direction is backwards to most conventions.
@joseph-robertson There's a minor merge conflict that I'll resolve. |
Pull request overview
Please read OpenStudio Pull Requests to better understand the OpenStudio Pull Request protocol.
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)src/openstudio_lib/library/OpenStudioPolicy.xml
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.