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

Multidimensional Newton operator #8

Closed
wants to merge 8 commits into from

Conversation

dpsanders
Copy link
Member

@dpsanders dpsanders commented Apr 28, 2017

Add a multi-dimensional Newton operator.

@dpsanders
Copy link
Member Author

Note that this requires changes to the @intervalbox macro.
That macro should probably be moved from IntervalArithmetic.jl to this package.

@dpsanders
Copy link
Member Author

If you have time to look at this one too, that would be great @lbenet.

As far as I remember, this follows on from the bisection one and so that should be merged first.

@lbenet
Copy link
Member

lbenet commented May 8, 2017

I'll take a look on this too, but hold me on about 30 min. A quick remark: tests are not passing.

@lbenet
Copy link
Member

lbenet commented May 8, 2017

That macro should probably be moved from IntervalArithmetic.jl to this package.

But then IntervalArithmetic.jl would be only of use for one-dimensional intervals. I do think that also the multidimensional definitions should be in IntervalArithmetic.jl, while applications should build upon that. Or am I missing something?

@dpsanders
Copy link
Member Author

I am not suggesting removing IntervalBox from IntervalArithmetic.jl -- I completely agree with you that it belongs there.

I am only talking about the @intervalbox macro that is wrapped around a function in order to automatically define the standard function that takes two variables (I think it is only for 2 variables currently?), as well as versions that act on IntervalBoxes and Arrays.

Maybe this is, however, not the correct solution, and rather a new function should be defined like I did for complex_bisection.

Copy link
Member

@lbenet lbenet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand why travis has problems with the tests with v0.6.

Regarding @intervalbox, I think this is not necessary at all (I'll post a comment on this separately). I think newton_refine needs to be extended as well.

@@ -0,0 +1,55 @@
# This file is part of the ValidatedNumerics.jl package; MIT licensed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file should be in IntervalArithmetic.jl; see this comment...

@@ -0,0 +1,56 @@
using ValidatedNumerics
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file should belong to `IntervalArithmetic.jl"; see above.

@lbenet
Copy link
Member

lbenet commented May 8, 2017

Regarding the necessity (or not) of @intervalbox, I'd like to point out that this already works!

julia> using ValidatedNumerics

julia> X = IntervalBox(1..1, 2..2)
[1, 1] × [2, 2]

julia> f(x, y) = (x + y, x - y)
f (generic function with 1 method)

julia> f(X...)
([3, 3],[-1, -1])

Maybe my concern with @intervalbox is that the name reminds me @interval, so I don't relate it with adpating a function, but with the creation of an IntervalBox.

@dpsanders
Copy link
Member Author

The difference is that @intervalbox defines a version of f such that f(X) takes an IntervalBox and returns an IntervalBox. I seem to have broken my VN installation though, so can't show it right now. (@intervalbox is not defined?)

@dpsanders
Copy link
Member Author

And to use ForwardDiff for automatic differentiation, as in this PR, we need a version that takes an Array as argument. It's all a bit of a mess.

@lbenet
Copy link
Member

lbenet commented May 8, 2017

The difference is that @intervalbox defines a version of f such that f(X) takes an IntervalBox and returns an IntervalBox. I seem to have broken my VN installation though, so can't show it right now. (@intervalbox is not defined?)

Sorry, I missed that. Still, I dislike the name though, because I think of a shortcut similar to @interval. In any case, it may be a good idea to have a macro of that kind, or as you proposed, perhaps a function.

In any case, the contests of src/multidim/intervalbox.jl include the definition of IntervalBox, as well as other general methods; that's what I was criticizing.

@dpsanders
Copy link
Member Author

Yes, the name is not great.

Ah yes, oops.

@codecov-io
Copy link

codecov-io commented May 13, 2017

Codecov Report

Merging #8 into master will decrease coverage by 3.29%.
The diff coverage is 12.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master       #8     +/-   ##
=========================================
- Coverage   82.18%   78.88%   -3.3%     
=========================================
  Files           5        6      +1     
  Lines         174      180      +6     
=========================================
- Hits          143      142      -1     
- Misses         31       38      +7
Impacted Files Coverage Δ
src/intervalbox_macro.jl 0% <0%> (ø)
src/newton.jl 73.77% <16.66%> (-6.94%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 620eed3...8595a51. Read the comment docs.

@lbenet
Copy link
Member

lbenet commented May 19, 2017

I think this could be merged, though it would be nice to add some tests.

@dpsanders
Copy link
Member Author

Closing in favour of #24.

@dpsanders dpsanders closed this Aug 2, 2017
@lucaferranti lucaferranti deleted the newton_operator_multidim branch November 29, 2022 16:43
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 this pull request may close these issues.

None yet

3 participants