Skip to content
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

Support function returning IntervalBox and only use them internally #107

Open
Kolaru opened this issue Jan 21, 2019 · 1 comment · May be fixed by #203
Open

Support function returning IntervalBox and only use them internally #107

Kolaru opened this issue Jan 21, 2019 · 1 comment · May be fixed by #203

Comments

@Kolaru
Copy link
Collaborator

Kolaru commented Jan 21, 2019

Currently, the functions passed to roots must return SVector which has several drawbacks:

  • Require one more explicit import from the user.
  • IntervalBox and SVector work in a quite similar manner, but are not equivalent, which may cause unexpected bugs. For example consider the following
IB = IntervalBox(emptyinterval(), emptyinterval())
SV = SVector(emptyinterval(), emptyinterval())

Here isempty(IB) evaluates to true while isempty(SV) evaluates to false. This caused some of the bugs I'm trying to fix in #93 (an update is coming soon).

Therefore, I advise that SVector should only be used when ForwardDiff is directly involved, i.e. deep inside the contractors.

It may be possible to support both the current behavior and function returning IntervalBox (converting to the latter internally) using fancy multiple dispatch, otherwise this would be a breaking change.

@dpsanders
Copy link
Member

Good point and good idea.

For ForwardDiff I think we just need

import ForwardDiff

ForwardDiff.gradient(f, X::IntervalBox) = ForwardDiff.gradient(f, X.v)
ForwardDiff.jacobian(f, X::IntervalBox) = ForwardDiff.jacobian(f, X.v)
ForwardDiff.hessian(f, X::IntervalBox) = ForwardDiff.hessian(f, X.v)

@OlivierHnt OlivierHnt linked a pull request May 31, 2024 that will close this issue
@OlivierHnt OlivierHnt linked a pull request Jun 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants