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

MultiBody.Joints.Constraints.UniversalJoint uses == on Real #1002

Closed
modelica-trac-importer opened this issue Jan 14, 2017 · 6 comments
Closed
Assignees
Labels
bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Milestone

Comments

@modelica-trac-importer
Copy link

Reported by jmattsson on 25 Feb 2013 08:57 UTC
Modelica.Mechanical.MultiBody.Joints.Constraints.UniversalJoint contains the equations:

      // Constraint equations concerning rotations
      if n_a[2]==1 and n_b[3]==1 then
        // Rotation order 1-2-3, 1 locked
        R_rel.T[3,2]=0;
        frame_a.t[2]=0;
        frame_b.t[3]=0;
        elseif n_a[3]==1 and n_b[2]==1 then
         // Rotation order 1-3-2, 1 locked
        R_rel.T[2,3]=0;
        frame_a.t[3]=0;
        frame_b.t[2]=0;
        elseif n_a[1]==1 and n_b[2]==1 then
         // Rotation order 3-1-2, 3 locked
        R_rel.T[2,1]=0;
        frame_a.t[1]=0;
        frame_b.t[2]=0;
        elseif n_a[2]==1 and n_b[1]==1 then
         // Rotation order 3-2-1, 3 locked
        R_rel.T[1,2]=0;
        frame_a.t[2]=0;
        frame_b.t[1]=0;
        elseif n_a[1]==1 and n_b[3]==1 then
         // Rotation order 2-1-3, 2 locked
        R_rel.T[3,1]=0;
        frame_a.t[1]=0;
        frame_b.t[3]=0;
        elseif n_a[3]==1 and n_b[1]==1 then
         // Rotation order 2-3-1, 2 locked
        R_rel.T[1,3]=0;
        frame_a.t[3]=0;
        frame_b.t[1]=0;
      else
        // No rotation
        {1,1,1}=R_rel.T * {1,1,1};
      end if;

The n_a and n_b vectors are of type Real, and the specification clearly disallows == and <> for Reals.

The rationale given in the specification for this limitation mostly concerns variables, so it might be reasonable to lift the restriction for parameters.


Migrated-From: https://trac.modelica.org/Modelica/ticket/1002

@modelica-trac-importer modelica-trac-importer added this to the MSL3.2.1 milestone Jan 14, 2017
@modelica-trac-importer modelica-trac-importer added bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody labels Jan 14, 2017
@modelica-trac-importer
Copy link
Author

Comment by sjoelund.se on 25 Feb 2013 10:44 UTC
I looked into why OpenModelica doesn't care about this. And yes, we only make the check for variables. For structural parameter there is no issue at run-time. And even for non-structural parameters... During initialization, there are no state events, right? ;)

If MSL wants to follow the specification, the code should be re-written or we need to schedule this for spec 3.2rev2. I'd propose the spec changes.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 25 Feb 2013 12:48 UTC
The model(s) needs to be rewritten.

The underlying reason for disallowing comparison between real variables is that one needs to consider that they are not exactly computed; this can occur during simulation but also due to the imprecise calculations for parameters.

In this case it is even more problematic(*) - there are two real vectors as input and only 6 combinations of values are supported for them (and then an else treating all other cases the same), i.e.

 if n_a[2]==1 and n_b[3]==1 then
    // Rotation order 1-2-3, 1 locked
    R_rel.T[3,2]=0;
    frame_a.t[2]=0;
    frame_b.t[3]=0;
   ...
  else
    // No rotation
    {1,1,1}=R_rel.T * {1,1,1};
  end if;

If you only support a limited set of values you use an integer or enumeration instead of testing for specific values of reals; that clearly documents it for users and avoids the problems.

The same goes for Constraints.RevoluteJoint
*: I haven't svn updated today in case it was recently changed

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 27 Feb 2013 08:30 UTC
Looking more it seems the correct solution is to remove all of the code above and replace it with something like (note that n_a and n_b have Evaluate=true so for the cases above it should generate the same result):

frame_a.t*n_a=0;
frame_b.t*n_b=0;
n_b*R_rel.T*n_a=0;
assert(abs(n_a*n_b)>Modelica.Constants.eps, "The axis must be different");

Assuming I did not misunderstand something.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 27 Feb 2013 08:33 UTC
Except that the assert should be on something like norm(cross(n_a,n_b)) instead of on abs(n_a*n_b) - or possibly assert that abs(n_a*n_b)<...

@modelica-trac-importer
Copy link
Author

Comment by otter on 3 Mar 2013 13:19 UTC
Fixed by Andreas Heckmann (the author) in 392e03f

@modelica-trac-importer
Copy link
Author

Comment by dietmarw on 3 Mar 2013 14:09 UTC
392e03f introduced this:

  //Modelica.Constants.eps 

why? Since its commented out it should never have been added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Projects
None yet
Development

No branches or pull requests

2 participants