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

Fix position prescribed friction #4256

Merged

Conversation

m-kormann
Copy link
Contributor

The fix from #4129 has been accidentally reverted by #4255 without being the reason for the compiling errors.

The compilation errors should be fixed by recompilation of the binaries as discussed in #3911, #4178 and #4250.

…x-friction"

This reverts commit ee5fae8, reversing
changes made to ae7afdc.
Copy link
Contributor

@DagBruck DagBruck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An improvement compared to the status-quo. As discussed in the earlier PR 4129.

Copy link
Member

@MartinOtter MartinOtter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have re-analyzed the solution and propose a slightly different fix. I would have included this fix in the pull request, but I don't manage to quickly do it (and do not have enough time to figure this out). So, someone else has to include this solution.

Issue with current proposal:

a_relfric/unitAngularAcceleration = if locked then 0 else if free then sa
else if startForward then sa - tau0_max/unitTorque
else if startBackward then sa + tau0_max/unitTorque
else sa - sign(w_relfric)*tau0_max/unitTorque;

Assume mode == Forward, so forward sliding, and wrelfric crosses zero. Then previously, the equation "mode = .... w_relfric > 0 ..." triggered a state event at w_relfric=0 and during finding the root w_relfric=0, all signals are continuous. With the change above, this is no longer the case, because sa and a_relfric are computed with "a_relfric = sa - sign(w_relfric)..." during root finding, so a_relfric and sa are discontinuous during root finding (which is bad).

This can be fixed by reformulating the equations to:
a_relfric/unitAngularAcceleration = if locked then 0 else if free then sa
else if startForward then sa - tau0_max/unitTorque
else if startBackward then sa + tau0_max/unitTorque
else if pre(mode) == Forward then sa - tau0_max/unitTorque
else if pre(mode) == Backward then sa + tau0_max/unitTorque
else sa - sign(w_relfric)*tau0_max/unitTorque;

The difference can be seen in the test example ModelicaTest.Rotational.TestFrictionPosition with a small number of output points ("number of intervals = 10" in Dymola).

Proposed solution
grafik

New solution:
grafik

@tobolar
Copy link
Contributor

tobolar commented Jan 12, 2024

I have re-analyzed the solution and propose a slightly different fix.

@MartinOtter I have implemented your suggestion in PR m-kormann#1
@m-kormann Please merge m-kormann#1 on your branch "revert-4255-revert-fix-friction" in order to be visible here.

Enhance a_relfric for forward/backward sliding
@MartinOtter MartinOtter merged commit 267ceba into modelica:master Jan 12, 2024
2 checks passed
@beutlich beutlich removed the request for review from HansOlsson January 12, 2024 18:31
@beutlich beutlich added L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational L: Mechanics.Translational Issue addresses Modelica.Mechanics.Translational labels Jan 12, 2024
@beutlich beutlich added this to the MSL4.1.0 milestone Jan 12, 2024
@beutlich beutlich changed the title Revert "Merge pull request #4255 from modelica/revert-4129-fix-friction" Fix position prescribed friction Jan 12, 2024
@m-kormann m-kormann deleted the revert-4255-revert-fix-friction branch January 15, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: Mechanics.Rotational Issue addresses Modelica.Mechanics.Rotational L: Mechanics.Translational Issue addresses Modelica.Mechanics.Translational
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants