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.

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

Deprecate Function::jacobian, Function::derivative #1777

Closed
jaeandersson opened this Issue Apr 29, 2016 · 8 comments

Comments

Projects
None yet
2 participants
@jaeandersson
Member

jaeandersson commented Apr 29, 2016

The functions for generating derivative functions in Function are currently difficult to use and, if kept, will need to have their signatures changed to align with the corresponding changes to the internal classes.

Since #1774 introduces a much easier to use and much more flexible way to generate derivatives, we can simply deprecate these functions instead of trying to update the signatures or maintain backwards compatibility. Already now, these functions are of internal character and avoided in examples.

@jaeandersson jaeandersson self-assigned this Apr 29, 2016

@jaeandersson jaeandersson added the task label Apr 29, 2016

@jaeandersson jaeandersson added this to the Version 3.1 milestone Apr 29, 2016

@jaeandersson jaeandersson changed the title from Deprecate Function derivative function generator to Deprecate Function::jacobian, Function::derivative Apr 29, 2016

jaeandersson added a commit that referenced this issue May 3, 2016

jaeandersson added a commit that referenced this issue May 3, 2016

jaeandersson added a commit that referenced this issue May 3, 2016

jaeandersson added a commit that referenced this issue May 3, 2016

jaeandersson added a commit that referenced this issue May 4, 2016

jaeandersson added a commit that referenced this issue May 4, 2016

jaeandersson added a commit that referenced this issue May 4, 2016

@jgillis

This comment has been minimized.

Member

jgillis commented May 10, 2016

make way, casadi 4.0 is here..

@jaeandersson

This comment has been minimized.

Member

jaeandersson commented May 10, 2016

make way, casadi 4.0 is here..

I think we have since long moved away from Function::jacobian in favour of working with expressions and using casadi::jacobian. For sure we have avoided it in all new examples and exercises. Working with expressions is just much more intuitive and in most circumstances at least as efficient.

jaeandersson added a commit that referenced this issue May 10, 2016

@jaeandersson jaeandersson modified the milestones: Version 3.2, Version 3.1 Oct 18, 2016

@jaeandersson

This comment has been minimized.

Member

jaeandersson commented Nov 21, 2016

@jgillis When this is implemented, parts of the unit tests will be obsolete. Preferred solution? Remove, refactor or replace with new unit tests?

jaeandersson added a commit that referenced this issue Nov 21, 2016

@jaeandersson

This comment has been minimized.

Member

jaeandersson commented Nov 21, 2016

Note that the idea is to use the (more user-friendly) functions operating on expressions instead:

MatType jacobian(const MatType &ex, const MatType &arg, bool symmetric=false);

Potentially, we could change the signature for these to take a dictionary:

MatType jacobian(const MatType &ex, const MatType &arg, const Dict& opts=Dict());

Then the user could set a parameter to force a particular AD mode for example:

J = jacobian(f, x, {'ad':'forward'}) # Force the use of forward mode AD

This feels more intuitive than setting "ad weight" parameters...

jaeandersson added a commit that referenced this issue Nov 22, 2016

jaeandersson added a commit that referenced this issue Nov 22, 2016

@jgillis

This comment has been minimized.

Member

jgillis commented Nov 22, 2016

Its fine if you purge, provided that you link to this issue. I will review

@jgillis

This comment has been minimized.

Member

jgillis commented Nov 22, 2016

This would be an alternative to ad_weight not a replacement?

@jgillis jgillis self-assigned this Nov 22, 2016

@jaeandersson

This comment has been minimized.

Member

jaeandersson commented Nov 22, 2016

This would be an alternative to ad_weight not a replacement?

I think I'd like to remove "ad_weight" as a user-settable option. It would remain internally and in Callback. The only time you would use it right now is to force an AD mode, but if there is a more user-friendly way of doing that, it's no longer needed.

jaeandersson added a commit that referenced this issue Nov 22, 2016

jaeandersson added a commit that referenced this issue Nov 22, 2016

jaeandersson added a commit that referenced this issue Nov 22, 2016

@jaeandersson

This comment has been minimized.

Member

jaeandersson commented Nov 22, 2016

@jgillis I deprecated Function::jacobian, Function::hessian in branch_1777: https://github.com/casadi/casadi/tree/issue_1777

There are so many unit tests that depend on them so just deleting them would leave the test framework severely crippled... Maybe you can have a look.

There are also some examples that have not been updated to 3.0 syntax:

  • docs/examples/python/lqr_control.py
  • docs/examples/python/simulation.py
  • docs/examples/python/mhe_spring_damper.py

jaeandersson added a commit that referenced this issue Nov 22, 2016

jaeandersson added a commit that referenced this issue May 21, 2017

jaeandersson added a commit that referenced this issue May 21, 2017

jaeandersson added a commit that referenced this issue May 22, 2017

@jgillis jgillis removed their assignment May 22, 2017

jaeandersson added a commit that referenced this issue May 22, 2017

jaeandersson added a commit that referenced this issue May 22, 2017

jaeandersson added a commit that referenced this issue May 26, 2017

jaeandersson added a commit that referenced this issue May 27, 2017

jaeandersson added a commit that referenced this issue May 27, 2017

jaeandersson added a commit that referenced this issue May 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment