-
Notifications
You must be signed in to change notification settings - Fork 169
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
Add unit="1" to quaternions #4397
Conversation
Note that Modelica.Mechanics.MultiBody.Frames.Quaternions.der_Orientation already have a corresponding unit. That has two implications: * It is the correct unit; as it cannot be anything else. * In many cases we could already deduce the unit for the quaternion based on its derivative. However, if we didn't compute in a specific model the derivative that wouldn't happen. Additionally it seemed a bit counter-intuitive to go that indirect route.
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.
Looks excellent to me. I'm approving with the caveat that we are currently not in a position where we can evaluate the implications for unit checking, since our implementation is currently not paying much attention to arrays.
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.
LGTM
What should I do regarding the CLA-check? |
Looks stuck. No idea why. |
@casella Can this be backported to maint/4.1.0? No Milestone either |
Please do. |
Note that Modelica.Mechanics.MultiBody.Frames.Quaternions.der_Orientation already have a corresponding unit. That has two implications: * It is the correct unit; as it cannot be anything else. * In many cases we could already deduce the unit for the quaternion based on its derivative. However, if we didn't compute in a specific model the derivative that wouldn't happen. Additionally it seemed a bit counter-intuitive to go that indirect route.
Backporting this to maintenance branch by #4445 |
Note that Modelica.Mechanics.MultiBody.Frames.Quaternions.der_Orientation already have a corresponding unit. That has two implications: * It is the correct unit; as it cannot be anything else. * In many cases we could already deduce the unit for the quaternion based on its derivative. However, if we didn't compute in a specific model the derivative that wouldn't happen. Additionally it seemed a bit counter-intuitive to go that indirect route. Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Note that Modelica.Mechanics.MultiBody.Frames.Quaternions.der_Orientation already have a corresponding unit. That has two implications:
However, if we didn't compute the derivative in a specific model that deduction wouldn't happen. Additionally it seems a bit counter-intuitive to go that indirect route.