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

Refactor Interval{Float64}^x #47

Closed
wants to merge 4 commits into from
Closed

Refactor Interval{Float64}^x #47

wants to merge 4 commits into from

Conversation

lbenet
Copy link
Member

@lbenet lbenet commented Jun 3, 2017

This PR uses only Float64 arithmetic to implement a method for ^(a::Interval{Float}, n::Integer), i.e., avoids using big53.

@coveralls
Copy link

coveralls commented Jun 3, 2017

Coverage Status

Coverage decreased (-0.1%) to 92.181% when pulling 833158d on refactor_power into b6f3521 on master.

@codecov-io
Copy link

codecov-io commented Jun 3, 2017

Codecov Report

Merging #47 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #47      +/-   ##
==========================================
- Coverage   92.31%   92.19%   -0.13%     
==========================================
  Files          21       21              
  Lines         924      999      +75     
==========================================
+ Hits          853      921      +68     
- Misses         71       78       +7
Impacted Files Coverage Δ
src/intervals/arithmetic.jl 98.75% <ø> (-0.02%) ⬇️
src/intervals/intervals.jl 96% <100%> (-4%) ⬇️
src/intervals/functions.jl 92.77% <100%> (-2.98%) ⬇️
src/intervals/rounding.jl 76.92% <0%> (+5.12%) ⬆️

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 b6f3521...e12392e. Read the comment docs.

@dpsanders
Copy link
Member

The problem is that the rounded versions of ^ still use BigFloat internally.

There is some kind of power available in CRlibm that I didn't wrap, but it is known that correctly rounding the power function is very difficult.

That is why the only solution I see is not to demand tight correct rounding.

@lbenet
Copy link
Member Author

lbenet commented Jun 3, 2017

The problem is that the rounded versions of ^ still use BigFloat internally.

This PR replaces the version that uses big53 in ^(a::Interval{Float64}, n::Integer) by operations purely with Float64, which is the case considered by pow. The tests pass, so I would say it is a question of benchmarking against the previous version and pow. This version provides tight bounds as far as the ITF1788 tests are concerned.

At the moment I am playing with ``^(a::Interval{Float64}, n::Float64)` to see how far can I get.

@coveralls
Copy link

coveralls commented Jun 3, 2017

Coverage Status

Coverage increased (+0.08%) to 92.395% when pulling 53a97ea on refactor_power into b6f3521 on master.

@lbenet
Copy link
Member Author

lbenet commented Jun 3, 2017

This is ready for review; test pass locally!

@lbenet lbenet changed the title Method for Interval{Float64}^n (pown) without big53 Refactor Interval{Float64}^x Jun 3, 2017
@lbenet lbenet requested a review from dpsanders June 3, 2017 21:46
@coveralls
Copy link

coveralls commented Jun 3, 2017

Coverage Status

Coverage decreased (-0.1%) to 92.192% when pulling e12392e on refactor_power into b6f3521 on master.

@dpsanders
Copy link
Member

I was referring to these lines, where ^(x, y, RoundDown) is defined in terms of BigFloats.

@lbenet
Copy link
Member Author

lbenet commented Jun 4, 2017

You are totally right; I forgot the changes in rounding.jl...

@lbenet
Copy link
Member Author

lbenet commented Jun 4, 2017

Closing this, to think this over...

@lbenet lbenet closed this Jun 4, 2017
@dpsanders dpsanders deleted the refactor_power branch August 2, 2017 19:40
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.

4 participants