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.

Sign up for GitHub

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

Jump to bottom

disable -fassociative-math (within -funsafe-math-optimizations) #12096

Merged
merged 1 commit into from
Jun 5, 2019
Merged

disable -fassociative-math (within -funsafe-math-optimizations) #12096

merged 1 commit into from
Jun 5, 2019

Conversation

dagar
Copy link
Member

@dagar dagar commented May 28, 2019 •
edited by AuterionWrikeBot
Loading

@dagar dagar requested review from jkflying and RomanBapst May 28, 2019 15:57
@dagar dagar mentioned this pull request May 28, 2019
Merged
@dagar
Copy link
Member Author

dagar commented May 28, 2019

Overall flash differences from current master on px4_fmu-v5.

image

@dagar dagar requested a review from a team May 28, 2019 15:59
@dagar
Copy link
Member Author

dagar commented May 28, 2019

@PX4/testflights could you give this one quick flight on a single platform with a comparison to current master?

@dagar
Copy link
Member Author

dagar commented May 28, 2019

Questions

  1. Does this result in any major performance regressions?
  2. Can we convince ourselves that this is the strictly safer thing to do?

@dagar dagar requested a review from priseborough May 28, 2019 18:59
@dagar dagar self-assigned this May 28, 2019
@jkflying
Copy link
Contributor

Note this also has the Kahan summation, so I expect the flight performance to be a little different, since the bias variance estimates will actually grow now unless there is data fused that reduces them. The CPU load will be particularly interesting to track. Note the places with big code size differences will probably also be the places with big performance differences - mostly stuff with matrix oeprations that can't have common calculations factored out or cancelled now. It depends if it is in a hot loop or not.

There are also some risks, for example the optimization would save us from 0/0 situations since the variables would be cancelled. We'll have to keep an eye out for this.

@jkflying
Copy link
Contributor

jkflying commented May 29, 2019 •
edited
Loading

SITL testing works fine for me, no noticeable changes.

@LorenzMeier
Copy link
Member

I would merge Kahan summation first, then evaluate the math flags separately.

@jkflying
Copy link
Contributor

The issue is that the Kahan summation is optimized out by the math flags, so is 'implicitly disabled' unless we do workarounds in the ECL.

@LorenzMeier
Copy link
Member

I'm aware, but I would do that locally first.

@Junkim3DR
Copy link

Junkim3DR commented May 29, 2019 •
edited
Loading

Tested on Pixhawk 4mini v5

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • Failsafe RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint turn off RC to trigger Failsafe RTL and see landing behavior.

Notes
No issues noted, good flight in general.

Logs

@dagar
Copy link
Member Author

dagar commented May 29, 2019

Unexpectedly the cpu usage actually decreased slightly.

Master - https://review.px4.io/plot_app?log=9cb9ecae-bdf7-49aa-968e-6c523a153220
image

PR 12096 - https://review.px4.io/plot_app?log=16427fc8-4db8-4f08-a525-895c51298e3f
image

@dannyfpv
Copy link

dannyfpv commented May 29, 2019 •
edited
Loading

Tested on Pixhawk 4 v5
Modes Tested: Acro mode

Notes: In Acro mode, while flying, it does not feel like full acro mode.
In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize.
Flying acro with other flight controllers would produce a smooth pass through this position.

https://review.px4.io/plot_app?log=5f79e673-f4f3-41e3-a1c1-da3205ca4437

flight card
1 https://review.px4.io/plot_app?log=c6420215-6a89-4bfc-9627-08ca39f9cb5b
2 https://review.px4.io/plot_app?log=e8a0ce0f-e3c9-4bd2-adb9-d73944528281
3 https://review.px4.io/plot_app?log=623e3b71-bc09-4029-b5f1-831f30703289
4 https://review.px4.io/plot_app?log=31925bfc-69a2-4b8e-92ee-7ee06909561c

Tested on Pixhawk1 v3 fixed wing
Modes Tested:

Position Mode: no issue
Altitude Mode: no issue
Stabilized Mode: no issue
mission mode: no issues

log:
https://review.px4.io/plot_app?log=8a44d93e-0421-44e6-8c09-8079e0567159

@dagar dagar changed the title [DO NOT MERGE]: consider changing math optimizations ecl/EKF Kahan summation and disable fassociative-math (within funsafe-math-optimizations) May 29, 2019
@dagar
Copy link
Member Author

dagar commented May 29, 2019

@PX4/testflights please exhaustively test this across all vehicles.

@jorge789
Copy link

jorge789 commented May 29, 2019 •
edited
Loading

Tested on PixRacer V4:

Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.
- Notes
No issues noted, good flight in general.

Logs

- PR 12096
https://review.px4.io/plot_app?log=aa1756f3-2994-4adc-bca0-2f0c271

Log Comparison to Master:
https://review.px4.io/plot_app?log=1c2fcafa-9b0f-41a2-8435-8c3350c8cfb8

Tested on PixRacer V4:

Modes Tested:
-Acro mode.

Procedure:
Armed and took-off in acro mode, while in the air performed flips and rolls for about a minute, then proceeded to land.

Notes: In Acro mode, while flying, it does not feel like full acro mode.
In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize.
Flying acro with other flight controllers would produce a smooth pass through this position.

Log:
https://review.px4.io/plot_app?log=a21269c1-081e-4379-8ce8-00c3841372bc

Tested on Pixhawk 2 Cube V3:

Modes Tested

Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.
Mission Plan Mode (Automated): Good.
RTL: Good.

- Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoin activate RTL and see landing behaviour.
- Notes
No issues noted, good flight in general.

Logs
https://review.px4.io/plot_app?log=c87d3e56-c34a-4562-af1e-3426e5ab4648

Log Comparison to Master:
https://review.px4.io/plot_app?log=29af6d61-61ac-49c6-a71e-df221eef0ac3

Tested on Pixhawk 2 Cube V3:

Modes Tested:
-Acro mode.

Procedure:
Armed and took-off in acro mode, while in the air performed flips and rolls for about a minute, then proceeded to land.

Notes: In Acro mode, while flying, it does not feel like full acro mode.
In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize.
Flying acro with other flight controllers would produce a smooth pass through this position.

Log:
https://review.px4.io/plot_app?log=18af06e1-a19c-4037-8932-fc77dc4e4330

@dagar dagar force-pushed the pr-math_opt_ecl branch from be45be9 to 0e5a068 Compare May 30, 2019 00:53
@Junkim3DR
Copy link

Tested on Pixhawk 4 mini v5

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.
  • Acro: Good.

Procedure

  • Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint trigger RTL and see landing behavior.

  • For Acro mode: Armed and took-off in acro mode, while in the air performed flips and rolls for about a minute, then proceeded to land.

Notes

  • No issues noted, good flight in general.
  • For Acro mode: While flying, it does not feel like full acro mode.
    In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize.
    Flying acro with other flight controllers would produce a smooth pass through this position.

Logs

Tested on Pixhawk Pro v4

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.
  • Acro: Good.

Procedure

  • Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint trigger RTL and see landing behavior.

  • For Acro mode: Armed and took-off in acro mode, while in the air performed flips and rolls for about a minute, then proceeded to land.

Notes

  • No issues noted, good flight in general.
  • For Acro mode: While flying, it does not feel like full acro mode.
    In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize.
    Flying acro with other flight controllers would produce a smooth pass through this position.

Logs

Tested on NXP FMUK66 v3

Modes Tested

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to altitude then stabilized mode proceed to switch to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint trigger RTL and see landing behavior.

Notes
Vehicle felt more responsive, smoother and more stable; good flight in general.

Logs

@dagar
Copy link
Member Author

dagar commented May 30, 2019

This is looking good so far. New plan, I'll update the PR to bring in only the optimization flag changes, which will then put PX4/PX4-ECL#601 in a position where they can bring in the change when @priseborough is ready.

@dagar dagar force-pushed the pr-math_opt_ecl branch from 0e5a068 to 01e6148 Compare May 30, 2019 13:42
@dagar dagar changed the title ecl/EKF Kahan summation and disable fassociative-math (within funsafe-math-optimizations) disable -fassociative-math (within -funsafe-math-optimizations) May 30, 2019
@dagar dagar marked this pull request as ready for review May 30, 2019 13:42
@jorge789
Copy link

jorge789 commented May 30, 2019 •
edited
Loading

Tested on PixRacer V4:

Flight Card 1

Modes Tested:
Position Mode: Good.
Altitude Mode: Good.
Stabilized Mode: Good.

Procedure:
Arm in position mode, check take off behavior (Vehicle should stay on the ground as long as the throttle stick is at the middle or below, and start to ascend smoothly as long as throttle stick as above the middle.), the land and disarm.
Same procedure was repeated with Altitude and Stabilized mode.

Notes:
No issues were noted, good flight in general.

Flight Card 2

Modes Tested
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Flight Card 3

Modes Tested
Position Mode: Good.
Mission Plan Mode (Automated): Good.
RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Logs:

Flight card 1: https://review.px4.io/plot_app?log=63ea5b4e-d8df-49cf-a25b-ad5495395a76

Flight card 2: https://review.px4.io/plot_app?log=7c6f8293-2489-4784-8823-df034c5d17a2

Flight card 3: https://review.px4.io/plot_app?log=e195ddd3-b27b-4d66-a010-1759d23809cf

Tested on Pixhawk 2 Cube V3:

Note: Lowered MPC_THR_MIN parameter from 12 to 11, no difference was seen due to wind not being strong enough for the paused landing issue to be reproduced. We will keep testing and trying to reproduce the issue.

Logs

Flight card 1: https://review.px4.io/plot_app?log=b8bce6d8-9e85-4ab3-915d-3169a818e967

Flight card 2: https://review.px4.io/plot_app?log=4de0f92b-5efc-47bf-a3c7-26df2b97e677

Flight card 3: https://review.px4.io/plot_app?log=bb4a041f-3118-4180-9859-e967b60527e8

@Junkim3DR
Copy link

Tested on Pixhawk Pro v4

Flight Card 1

Modes Tested:

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.

Procedure:
Arm in position mode, check take off behavior (Vehicle should stay on the ground as long as the throttle stick is at the middle or below, and start to ascend smoothly as long as throttle stick as above the middle.), the land and disarm.
Same procedure was repeated with Altitude and Stabilized mode.

Notes:
No issues were noted, good flight in general.

Flight Card 2

Modes Tested

  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.

Procedure
Arm and Take off in mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Flight Card 3

Modes Tested

  • Position Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Flight Card 4

Modes Tested

  • Position Mode: Good.
  • Failsafe RTL (Return To Land): Not returning after RC shutdown.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, turned RC off in order to activate failsafe RTL, wait for vehicle to return and land as expected.

Note:
No issues were noted, good flight in general.

Logs:

Tested on Pixhawk 4 mini v5

Flight Card 1

Modes Tested:

  • Position Mode: Good.
  • Altitude Mode: Good.
  • Stabilized Mode: Good.

Procedure:
Arm in position mode, check take off behavior (Vehicle should stay on the ground as long as the throttle stick is at the middle or below, and start to ascend smoothly as long as throttle stick as above the middle.), the land and disarm.
Same procedure was repeated with Altitude and Stabilized mode.

Notes:
No issues were noted, good flight in general.

Flight Card 2

Modes Tested

  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.

Procedure
Arm and Take off in mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Flight Card 3

Modes Tested

  • Position Mode: Good.
  • Mission Plan Mode (Automated): Good.
  • RTL (Return To Land): Good.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, switched to mission plan mode then make sure that vehicle follows all waypoints as shown in QGC, once completed all waypoint see landing behavior.

Note:
No issues were noted, good flight in general.

Flight Card 4

Modes Tested

  • Position Mode: Good.
  • Failsafe RTL (Return To Land): Not returning after RC shutdown.

Procedure
Arm and Take off in position mode, after flying for approximately one minute, turned RC off in order to activate failsafe RTL, wait for vehicle to return and land as expected.

Note:
No issues were noted, good flight in general.

Logs:

@dagar dagar removed the EKF2 label May 31, 2019
@dannyfpv
Copy link

dannyfpv commented May 31, 2019 •
edited by Tony3dr
Loading

Tested on Pixhawk 4 mini v5 v-tol convergence

Modes Tested:

Position Mode: perfect
Stabilized Mode: perfect
transition: perfect
log:
https://review.px4.io/plot_app?log=a5c65240-61c9-40dd-872f-96a761cf7cdf

mission mode: perfect
transition: perfect
land: perfect
log:
https://review.px4.io/plot_app?log=22a506bb-1eec-401e-9764-e945440bc36d

@weekly-digest weekly-digest bot mentioned this pull request Jun 2, 2019
Closed
@jkflying
Copy link
Contributor

jkflying commented Jun 4, 2019

@PX4/testflights can you do another acro test with the latest build? The Kahan stuff has been removed, I want to see if that fixes the feeling of stabilization when exactly horizontal in acro. It needs to be compared to master to see if it feels different.

@dagar dagar force-pushed the pr-math_opt_ecl branch from 01e6148 to c27afc4 Compare June 4, 2019 13:25
@dannyfpv
Copy link

dannyfpv commented Jun 4, 2019 •
edited by Tony3dr
Loading

Tested on Pixhawk pro V4:

Modes tested: Acro mode

Notes: In Acro mode, while flying, it still does not feel like full acro mode. In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize. Same behavior as before noted, no improvements noted.
Tested on Master Acro mode the feeling is better but still feels like it has assistance.
Flying acro with other flight controllers would produce a smooth pass through this position.

PR Acro mode test log:
https://review.px4.io/plot_app?log=ba7bce05-4718-4ff8-bec9-8404683d823e
https://review.px4.io/plot_app?log=21ec7361-08e6-4684-8835-b0fbc13bdcbb

Master test log:
https://review.px4.io/plot_app?log=1fdeec0d-604f-473f-8e63-e40ff6e5d316

Tested on Pixhawk4 mini v5
Modes Tested: Acro mode

Notes: In Acro mode, while flying, it feels much better than before, but still it does not feel like full acro mode. You can still note slight assistance to attempt to stabilize. It definitely improved, noted lock-in when rotating.
On master, Acro mode is better but you can still note slight assistance.

PR Acro mode test log:
https://review.px4.io/plot_app?log=ce154708-36a4-4917-b5ba-efdcd25b40f2
https://review.px4.io/plot_app?log=daee9d62-2a99-45ea-8421-91a7d706359d

Master test log:
https://review.px4.io/plot_app?log=c11dda1a-4632-4656-be76-7a75a2faf964

@bkueng
Copy link
Member

bkueng commented Jun 5, 2019

Notes: In Acro mode, while flying, it still does not feel like full acro mode. In rotation, the vehicle hesitates when passing through its level upright position in what seems like an attempt to stabilize. Same behavior as before noted, no improvements noted.

@dannyfpv please enable Airmode and let me know if it still feels off. The rate tracking is also not great on the log I see. Can you tune it? Make sure to increase the I gains.

jkflying
jkflying approved these changes Jun 5, 2019
View reviewed changes
Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

Based on the comparison of the logs between master and this PR I don't think there are any issues specifically caused by removing this optimization.

@LorenzMeier LorenzMeier merged commit 1d56848 into master Jun 5, 2019
@LorenzMeier LorenzMeier deleted the pr-math_opt_ecl branch June 5, 2019 12:54
@Tony3dr Tony3dr mentioned this pull request Jun 5, 2019
Closed
@weekly-digest weekly-digest bot mentioned this pull request Jun 9, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LorenzMeier LorenzMeier LorenzMeier approved these changes

@jkflying jkflying jkflying approved these changes

@RomanBapst RomanBapst Awaiting requested review from RomanBapst

@priseborough priseborough Awaiting requested review from priseborough

Assignees

@dagar dagar

Labels
Admin: Enhancement (improvement) 💡
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dagar @jkflying @LorenzMeier @Junkim3DR @dannyfpv @jorge789 @bkueng