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

fixed doctest issue #215

Merged
merged 3 commits into from
May 18, 2021
Merged

fixed doctest issue #215

merged 3 commits into from
May 18, 2021

Conversation

RohitRathore1
Copy link
Contributor

No description provided.

@RohitRathore1
Copy link
Contributor Author

It resolves issue #216

Copy link
Member

@jverzani jverzani left a comment

Choose a reason for hiding this comment

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

This is great. Thanks so much! There are a few things to sort out. Do you want to look into them? If not, I can.

@@ -366,15 +366,15 @@ dfᵏs (generic function with 1 method)

```jldoctest roots
julia> Roots.newton(f, D(f), 2)
2.0945514815423265
3.141592653589793
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, have to see what happened here... That difference isn't a numerical one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me check again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. By mistake I had changed the f

src/find_zero.jl Outdated
julia> fn = x -> (2x*cos(x) + x^2 - 3)^10/(x^2 + 1);

julia> x0, xstar = 3.0, 2.9947567209477;

julia> find_zero(fn, x0, Order2()) ≈ xstar
true
false
Copy link
Member

Choose a reason for hiding this comment

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

This one is odd too, as locally it works for me:

julia> fn = x -> (2x*cos(x) + x^2 - 3)^10/(x^2 + 1);

julia> x0, xstar = 3.0,  2.9947567209477;

julia> find_zero(fn, x0, Order2()) ≈ xstar
true

julia> find_zero(fn, x0, Order2()), xstar
(2.9947567209477, 2.9947567209477)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my environment, it doesn't work. Had you tried in 1.6.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


```

Or, for Halley's method:

```jldoctest roots
julia> Roots.halley(f, D(f), D(f,2), 2)
2.0945514815423265
3.141592653589793
Copy link
Member

Choose a reason for hiding this comment

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

This must be the same issue. I wonder if D or f is not matching the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me chack again.

@RohitRathore1
Copy link
Contributor Author

This is great. Thanks so much! There are a few things to sort out. Do you want to look into them? If not, I can.

Yeah, please tell me? I can.

@jverzani
Copy link
Member

jverzani commented May 13, 2021 via email

@RohitRathore1
Copy link
Contributor Author

This one might have floating point issues. I'm testing on v"1.6.0" on a mac. What about you?

On Thu, May 13, 2021 at 7:33 AM Rohit Singh Rathaur < @.**> wrote: @RohitRathore1 commented on this pull request. ------------------------------ In src/find_zero.jl <#215 (comment)>: > julia> fn = x -> (2xcos(x) + x^2 - 3)^10/(x^2 + 1); julia> x0, xstar = 3.0, 2.9947567209477; julia> find_zero(fn, x0, Order2()) ≈ xstar -true +false In my environment, it doesn't work. Had you tried in 1.6.0? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#215 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADG6TEZ7HKBSDPQNQ4ZGH3TNO2HZANCNFSM442I2HDQ .
-- John Verzani Department of Mathematics College of Staten Island, CUNY

Yeah, now it has resolved. The test case has also passed.

@jverzani
Copy link
Member

Thanks again! Once CI passes, I'll merge in.

@codecov
Copy link

codecov bot commented May 13, 2021

Codecov Report

Merging #215 (d957db1) into master (8a5ff76) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #215   +/-   ##
=======================================
  Coverage   85.02%   85.02%           
=======================================
  Files           9        9           
  Lines        1896     1896           
=======================================
  Hits         1612     1612           
  Misses        284      284           
Impacted Files Coverage Δ
src/find_zero.jl 80.46% <ø> (ø)
src/find_zeros.jl 95.83% <ø> (ø)

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 8a5ff76...d957db1. Read the comment docs.

@jverzani
Copy link
Member

HI @RohitRathore1 I'm seeing this error (https://github.com/JuliaMath/Roots.jl/pull/215/checks?check_run_id=2576725090) Can you address? If not, I'll pick it up. If you do, would you also bump the version number? Then we can merge and tag. Thanks.

@RohitRathore1
Copy link
Contributor Author

HI @RohitRathore1 I'm seeing this error (https://github.com/JuliaMath/Roots.jl/pull/215/checks?check_run_id=2576725090) Can you address? If not, I'll pick it up. If you do, would you also bump the version number? Then we can merge and tag. Thanks.

Yeah, I can. What do you mean by bump the version number?

@jverzani
Copy link
Member

In the Project.toml file change version = "1.0.9" to version = "1.0.10". That was we can tag a new release. (The nice additions are seen through juliahub which will re-run the documentation when a new version is released.)

@RohitRathore1
Copy link
Contributor Author

In the Project.toml file change version = "1.0.9" to version = "1.0.10". That was we can tag a new release. (The nice additions are seen through juliahub which will re-run the documentation when a new version is released.)

ok.

@jverzani
Copy link
Member

CI is still showing an issue. Is this an easy fix? If so, I'll wait. Otherwise, I can merge in and sort out.

@RohitRathore1
Copy link
Contributor Author

CI is still showing an issue. Is this an easy fix? If so, I'll wait. Otherwise, I can merge in and sort out.

Yes, it's easy to fix. You can wait. I will fix it.

@jverzani jverzani merged commit 3db7ad0 into JuliaMath:master May 18, 2021
@jverzani
Copy link
Member

Thanks again for your efforts here! So very much appreciated.

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

2 participants