System id improvements #4389
System id improvements #4389
Conversation
|
@lthall @bnsgeyer please review this. The results are very interesting! @Hwurzburg I have no permissions to add reviewers, nor labels here :( |
|
Can someone label this DevCall ? |
done |
|
Hey Henry, can you also approve the workflow? |
| | Disturbance Rejection | see graph below |not applied | Smaller is better | | ||
| +------------------------------------+-----------------+---------------+----------------------+ | ||
|
|
||
| The frequency-based disturbance rejection curve in decibel is shown below. |
There was a problem hiding this comment.
Wondering where you got the desired criteria? A disturbance rejection bandwidth (-3 db crossing) of 10 hz seems high. Was that scaled for your aircraft size?
There was a problem hiding this comment.
We decided on the requirements values ourselves.
Yes, these requirements are for a specific craft. Other craft need other values.
| +-------------------------------------------+-----------------------+----------------------+------------------------+ | ||
| | Parameter | Default | Optimized | Autotune | | ||
| +-------------------------------------------+-----------------------+----------------------+------------------------+ | ||
| | :ref:`ATC_RAT_RLL_P<ATC_RAT_RLL_P>` | 0.1350 | 0.123 | 0.240025 | |
There was a problem hiding this comment.
can I see a log of it flying with I = 8x P ?
There was a problem hiding this comment.
I believe this is caused by the high disturbance rejection criteria. I would think this could have some implications with integrator management during sloped takeoffs and landings.
|
Other concerns:
|
|
@tridge here is an aggressive flight with FF disabled: https://we.tl/t-e0guD0oDoC FF_ENABLED does cause small overshoot on the pitch axis, but not on the roll. |
| -------------------------------------- | ||
|
|
||
| The next four plots show the bode plot of the closed-loop system the simulation results for the tracking behavior of a 10 degree step, the normalized controller output corresponding to the angle step as well as the disturbance behavior for the roll axis. | ||
| The normalized controller output evaluates whether the optimized behavior can actually be realized by the real system. |
There was a problem hiding this comment.
I would suggest you do a better job at describing how the plot shows that the optimized behavior can realized by a real system. It's not obvious to me.
|
|
||
| Tracking Behaviour: | ||
|
|
||
| .. image:: ../images/rollAxisTrackingSim.png |
There was a problem hiding this comment.
I find it surprising that the autotune produced a response that was so oscillatory. I don't think that would be acceptable. @lthall any thoughts why the autotune would have resulted with a tune that was so oscillatory? I would think they could do better with the autotune feature.
There was a problem hiding this comment.
In my opinion this is the main problem with this page. The response of autotune is obviously not based on a good autotune OR the system identification is incorrect.
The results of a successful system identification and optimisation should result in tuning values similar to autotune based on your requirements.
A good autotune should have responses similar to your optimised response. (autotune generates step responses and adjusts gains based on overshoot and the first minimum so it is obvious that the responses shown here are inconsistent with autotune)
There was a problem hiding this comment.
The simulation results are based on an attitude controller that was simplified quite a bit since I didn't have time left at the end of my master thesis to realize a proper reconstruction of the controller structure. As an example, the acceleration limits were not implemented in order to linearize the controller for the model-based design. The improvement of the controller used for the simulation will be done in the upcoming work, so we can achieve more reliable simulation results. Additionally, we can provide plots that compare the real tracking behaviour of both controllers.
@lthall I don't fully agree that the requirements for the model-based tune should necessarily result in a similar tune. To my understanding, autotune is evaluating the resulting tune based on the tracking behaviour alone. Contrary to this, the model-based approach considers stability and disturbance behaviour as well. Since these are conflicting design targets, the resulting parameters are likely to differ from the autotune approach. Also, we allow different values for the P and I part of the controller, so the optimization has more freedom in finding good parameter values based on the given requirements.
There was a problem hiding this comment.
I said similar, not identical. The response should be very close too. You are using the same overshoot requirements and a fast step response time requirement. These two goals will pull the result towards the autotuned result. Phase and gain margin may be a little different to the autotune result but I am guessing that it will be similar (this is one of the things I am interested in seeing how they vary). Again, disturbance rejection isn't going to be pushing anything to far away from the autotuned result.
Step response stationary error should be zero unless I don't understand what that is. I am not confident I do understand what this is. (all errors should decay to zero)
I don't know what "Control Variable Max. Overshoot" is.
The only significant difference I can see between the two will be the different requirement for the I term. Again, this is one of the things I am interested to see how they differ.
|
|
||
| Bode plot of the closed-loop system: | ||
|
|
||
| .. image:: ../images/rollAxisClosedLoopBode.png |
There was a problem hiding this comment.
could you add to the graph or in the text what the input (target roll attitude?) and the output (actual roll attitude?) would be for the bode plot?
| +-------------------------------------------+-----------------------+----------------------+------------------------+ | ||
| | :ref:`ATC_RAT_RLL_FF<ATC_RAT_RLL_FF>` | 0 | 0 | 0 | | ||
| +-------------------------------------------+-----------------------+----------------------+------------------------+ | ||
| | :ref:`ATC_RAT_RLL_FLTT<ATC_RAT_RLL_FLTT>` | 23.0000 | 50.256 | 5.0 | |
There was a problem hiding this comment.
How did FLTT get changed in autotune. As far as I can tell from the code, this is not a filter that is changed during multicopter autotune. Is this a typo?
There was a problem hiding this comment.
It's not changed in autotune, it is switched off during twitches however
There was a problem hiding this comment.
As I said above, the structure of the attitude controller was simplified to get a linear controller structure for the model-based design. Because of this, the autotune parameters actually led to an unstable behaviour. In my opinion, the reason for this are the missing acceleration limits of the simplified controller. Therefore, I manually adjusted the target filter FLTT to achieve something like a linear limitation of the target value, which then led to a stable behaviour, but has implications on the controller performance because of latencies.
As you can see, we took some shortcuts in order to provide these results. We'll work on reconstructing the actual controller in the simulation to realize more reliable results that we can show here.
There was a problem hiding this comment.
I don't believe this is true. Autotune is run in the linear region of the attitude controller. It has step size limits that attempt to ensure this is the case. The step test does not apply any acceleration limits or other input shaping / control model. In operation the acceleration limits are there to ensure the aircraft is safe when it is recovering from large angle errors. In normal operation and mauvers the acceleration limit in the attitude controller should not be reached.
It looks like you had to reduce the FLTT to 5 Hz to get the autotune results to be stable. That seems very strange to me.
It is possible that your system ID settings may have pushed the aircraft into an acceleration limited region and that has resulted in an inaccurate model. However, if the model is based on the rate output -> gyro measurement then any controller limits or command model should not impact it. Maybe if the outputs are saturating, that might mess up the system ID result.
The key point is Autotune is designed to optimise the aircraft in the linear region, just as would expect your system ID model to represent. If your model is not stable with the autotuned results, with the same filter settings, then the model is not representative of the aircraft.
|
added devcall topic since it apparently never got discussed in the EU call.... @andyp1per or whoever runs those, you should look for labels in all repos, not just code |
|
@Hwurzburg we don't normally look at wiki issues - but can do. @tridge is usually the one with the list |
|
@tridge this was marked for DEVEU call a while back...I marked it for DEVCALL...Craig alwasy scans the all the repos for call labels |
|
I think we just need to make sure that this is something users can put to practical use. Certainly some good work has been done by @amilcarlucas and team but I think that perhaps this should be moved to a blog post or discuss topic and then perhaps link it from this system-id wiki page? My concerns is just that this is going to lead to issues and support questions and if these were posted in a discuss forum then it would be more convenient for amilcar to respond to. |
|
|
@IamPete1 suggested this should be linked from the sysid flight mode https://ardupilot.org/copter/docs/systemid-mode.html |
@amilcarlucas I didn't get a chance to download and view your files that you provided in your previous post and the links expired. can you provide the new links to these files? Thanks! |
@CraigElder this repo also has that label |
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
d42d0c3 to
d63dd97
Compare
|
I had all those labels defined with :name: directives. But you told me to remove those. |
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
f95a474 to
40d8af7
Compare
@amilcarlucas remove the reference to SID_AXIS types 1,2 and 3 from the current figure with ATC_RATE_FF_EN = false. Add the figure with ATC_RATE_FF_EN = true and place the reference to the SID_AXIS types 1, 2, and 3 as an injection in the same location as it was on the other diagram. You could add the other SID_AXIS types 7-13 in there respective places on this diagram as well since they would apply in both cases. @lthall do you understand my concern with the current diagram in this PR? |
|
This PR has now two diagrams. One with ATC_RATE_FF_EN = false and a second one with ATC_RATE_FF_EN = true. |
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
40d8af7 to
8620a06
Compare
|
Changes done |
|
@amilcarlucas i just looked and realized that you added the ATC_RATE_FF_EN = true plot. Ok so i think i understand my confusion. I am guessing the reason this may be considered accurate is due to the fact that the user could set either the pilot input or recovery SID_AXIS types regardless of what their ATC_RATE_FF_EN parameter is set because the mode adjusts the parameter to get the desired input. So my vote is to remove the SID_AXIS types 1, 2, and 3 from the diagram with the ATC_RATE_FF_EN =false and remove the SID_AXIS types 4, 5, and 6 from the diagram with the ATC_RATE_FF_EN = true. That would show how the control laws look when each axis type is used. |
|
done. |
|
@amilcarlucas yes. I concur with the most recent change. Thanks |
I will fix and then merge since BillG approved when I get a chance |
|
@amilcarlucas I just spent two hours working on this and have many more to do, which I cannot afford now....there are large numbers of build errors.....the figure titling/cross link methods are wrong and he relies on many many cross refs to figures that dont work....also the "math" tags do not work....this will have to wait for a while....sorry |
|
Thanks @Hwurzburg. Can you push your changes, or do a PR against fbredeme repository? |
Hwurzburg
force-pushed
the
pr-system-id-improvements
branch
from
8620a06 to
9bf2613
Compare
|
done...each figure ref needs a preceding tag..the math tags may have to be removed....but all these links have to be added and changed: |
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
2 times, most recently
from
9df1dae to
5867283
Compare
|
I fixed all that I could and force pushed it. |
|
Yes, you can use $ to denote inline math in reStructuredText (RST). However, please note that this is not standard RST syntax, but a feature provided by some tools like Sphinx. For example, you can write However, if you're getting a warning about the latex command not being able to run, it might be because LaTeX is not installed on your system or it's not in your system's PATH. If you want to use LaTeX for math rendering, you might need to install it or fix your PATH. If you don't want to install LaTeX and you're using Sphinx, you might want to switch to using MathJax for rendering math by adding these lines to your conf.py: This will render math using JavaScript in the HTML output, without needing LaTeX. Would that be an option? |
|
My local build environment is supposed to exactly mirror the server's...so either the environment setup scripts need changing, or this will generate build server errors..until this is resolved @TunaLobster @peterbarker I am not comfortable merging All the other errors are now gone thanks, and other than this issue it could be merged |
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
3 times, most recently
from
65beb66 to
0767c88
Compare
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
0767c88 to
621e8cc
Compare
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
621e8cc to
fc9cae5
Compare
amilcarlucas
force-pushed
the
pr-system-id-improvements
branch
from
fc9cae5 to
2c5a256
Compare
Follow-up pull request #4110 after internal review.