Closed manumerous closed 3 years ago
Could you also share quantitative results on how this change effects the overall pipeline? I am slightly confused what the intention of this PR is (if it is to merge or simply a proposal)
Could you also share quantitative results on how this change effects the overall pipeline? I am slightly confused what the intention of this PR is (if it is to merge or simply a proposal)
* It looks as if though the new optimizer is not enabled by default, therefore I am not sure what we gain from merging the PR * The wing model is modified in this PR, but the suggested advantages (e.g. enforcing constraints for stall dynamics) doesn't seem to be enabled. Adding stall dynamics parameters as part of the optimization is a strict blocker for fixedwing. We need to either model it as known dynamics or shouldn't add any of the dynamics to the pipeline at all.
Yes I intend to merge this PR. Have you read the descrition? There it is mentioned that it is implemented for the standardplane and quadrotor models. For both models the estimator is directly chosen in the config file. Have a look here as an example. There you can also see that the post stall coefficients are fixed.
@manumerous Okay thansks for clarifying - then how does this change the estimation perfomance - What does marginally worse performance mean?
It was observed that there was a case (resample_req=100) for the standard plane model where the optimizer did not converge.
These are the comments that I am most concerned about. I understand this PR is conceptually better, but not sure if it actually helps solving the problems in the pipeline.
Both model estimates lost/gained less than a promille in R2 and RMSE.
i mean this PR is the big step we talked about for quite a while now :) The new optimizer still minimizes the quadratic prediciton error while taking the constraints we give him into account. If the constraints of the coefficients are set in a smart way it actually gives you feedback whether the coefficients are at the bounds or not. This way we can ensure that we do not fit any nonphysical parameters (e.g. negative drag coefficient) and raise a warning to the user that his data does not result in a meaningful parameter.
In that sense this PR does not solve all the problems we have but it can lay the foundations for giving meaningful user feedback.
Furthermore we now have an abstract class that makes it easier to add new optimizers in the future (e.g. nonlinear scipi optimizer I used for the tiltwing).
All previous review comments should now be adressed
Read carefully!
Structure
This PR completely reworks the optimizer structure of the project:
Model Adaptions
Furthermore it adds the new QP-optimizers and parameter bounds for the quadrotor and standard plane model. Please have a close look at those bounds and help me improve them, I was not always sure about the best bounds.
Results
Both models only have marginally worse prediction metrics than their unbounded counterparts while managing to enforce the set constraints. With this it was able to enforce the right sign of multiple parameters (as for example a negative inflow scalar for the motor leaver moment: a positive parameter would mean a larger moment the larger the incoming perpendicular airspeed which might lead to instability).
Limitations
It was observed that there was a case (resample_req=100) for the standard plane model where the optimizer did not converge. For smaller dataset (starting from resample_req=50) it did however converge nicely. My suspicion here is that gradient based methods will always jump back and forth for the fixed parameters (eg min=1, max=1), which might lead to instability. I already have a solution in mind which eliminates those variables from the optimizer. but I guess that's something for another PR.