Repeated code in model classes

[Jaeyoung-Lim]

Problem Description This is just a note while looking into the architecture

While dynamics_model is a parent class of different models, there seems to be quite some code that are identical, but unshared between models.

e.g. estimate_model seems almost identical across models, but not quite. https://github.com/ethz-asl/data-driven-dynamics/blob/e0834d9aacb6a1a785ef918fda3f3f5e90658d18/Tools/parametric_model/src/models/quad_plane_model.py#L95-L112

https://github.com/ethz-asl/data-driven-dynamics/blob/e0834d9aacb6a1a785ef918fda3f3f5e90658d18/Tools/parametric_model/src/models/delta_quad_plane_model.py#L66-L83

https://github.com/ethz-asl/data-driven-dynamics/blob/e0834d9aacb6a1a785ef918fda3f3f5e90658d18/Tools/parametric_model/src/models/simple_quadrotor_model.py#L78-L90

[manumerous]

Thanks for bringing this up.

Currently i imagine the project to go in the direction that other people can make their own model class depending on their need of their project. For this it would be great if people could just choose their own optimization and fitting process, while all the interfaces to the other parts of the code are covered by the DynamicsModel class.

For example it should be fairly easy to switch the linear regression for e.g. a ridge regression, scipy.opimize or cvxpy. Since those methods might require a slightly differently arranged structure and data I thought it would be the fastest option to just allow the user to construct its own model class here.

But maybe it might be nicer to move some default optimization like the linear regression into the dynamics model at some point.

[Jaeyoung-Lim]

@manumerous Right, and this can always be done by overriding estimate_model function by choice. However, IMHO, the esitmation method should not be in the lower hierarchy than the model. e.g. you should be able to run your own estimation method with different models, and not need to create different models to run different optimization methods.

Given that said, I think the fact that the three are doing exactly the same thing, but are slightly different just in the syntax points to that this is not the most maintenance friendly way to structure the model.

[manumerous]

Yes i think in the case of the linear regression model this would be very easy and meaningful to do. In case of using the nonlinear optimizer in scipy.optimize it is slightly less straight forward due to separate cost and constraint functions specific to your model. Here i thought it would make sense to keep these methods as properties of the model they correspond to.

[manumerous]

There are also a lot of printouts in the estimate model method for example. I think at some point the hole user feedback via the terminal should be revisited and improved. Currently it still has quite some artifacts from debugging ect.