ardupilot icon indicating copy to clipboard operation
ardupilot copied to clipboard

AP_NavEKF3: document provenance of Python-generated equations

Open tpwrules opened this issue 1 year ago • 6 comments

Over time the current iteration of the generator has drifted from the actual code, both due to changes within itself and in the used libraries. This PR tracks down how that's happened and makes everything consistent again so that informed improvements can be made.

  • Code generator is modified to automatically use sq and friends.
  • Old generator versions that generate the current code have been dug up.
  • Nix shebangs are added to conveniently use the correct versions of the generator libraries.
  • Outdated previously generated code is removed from the repo.
  • Unused generated code is disabled.

No compiler output change (on CubeOrange at least).

tpwrules avatar Aug 17 '24 19:08 tpwrules

I think this looks great, needs approval by @priseborough

tridge avatar Aug 20 '24 00:08 tridge

LGTM

rmackay9 avatar Aug 20 '24 00:08 rmackay9

My goal with this PR was to minimize changes to the code and just document things as they are. Currently the results are indeed manually copied, and hopefully the steps should be relatively obvious now that the generated code matches what's actually being used.

We discussed in the dev call and indeed I think everyone would like to just run the generator and have the generated code committed (as the generator is pretty slow) and just include it. The current way is definitely not desirable, but improving it is down the road.

tpwrules avatar Aug 20 '24 19:08 tpwrules

My goal with this PR was to minimize changes to the code and just document things as they are. Currently the results are indeed manually copied, and hopefully the steps should be relatively obvious now that the generated code matches what's actually being used.

We discussed in the dev call and indeed I think everyone would like to just run the generator and have the generated code committed (as the generator is pretty slow) and just include it. The current way is definitely not desirable, but improving it is down the road.

Understood. Thanks for taking the time to answer!

Georacer avatar Aug 20 '24 20:08 Georacer

i think we should try to use one version of sympy for all generation, and check it generates good code, then assert sympy version instead of using nix-shell

Moved the nix stuff to its own file and added the version asserts. Two different versions are still required. Would prefer to change the code in a separate PR, though I still need to examine how big the change will be.

tpwrules avatar Aug 20 '24 21:08 tpwrules

Found a version of Sympy (1.9) that generates code identical to the current for both scripts, so only one version needs to be installed. Both scripts check for that version now.

tpwrules avatar Aug 24 '24 04:08 tpwrules