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.

Sign up for GitHub

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

Jump to bottom

Add TaylorN constructor #355

Merged
merged 4 commits into from
Apr 17, 2024
Merged

Add TaylorN constructor #355

merged 4 commits into from
Apr 17, 2024

Conversation

PerezHz
Copy link
Contributor

@PerezHz PerezHz commented Apr 10, 2024 •
edited
Loading

This PR adds a TaylorN constructor which takes simply a <:NumberNotSeries variable and the current order from get_order(), which allows to do e.g.:

julia> a = TaylorN{Float64}(-1//3)
 -0.3333333333333333 + 𝒪(‖x‖⁷)

julia> typeof(a)
 TaylorN{Float64}

@coveralls
Copy link

coveralls commented Apr 10, 2024 •
edited
Loading

Coverage Status

coverage: 96.958% (-0.001%) from 96.959%
when pulling 6d04fd7 on PerezHz:jp/taylorn
into 87116db on JuliaDiff:master.

@PerezHz PerezHz mentioned this pull request Apr 13, 2024
Merged
4 tasks
lbenet
lbenet reviewed Apr 15, 2024
View reviewed changes
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.

Thanks a lot for this addition (and sorry for the time to send my review). In general, LGTM, though I have included a comment requesting to add another tests. Aside from that, once that is implemented, I'm happy to merge it.

test/manyvariables.jl
Comment on lines +9 to +11
a = TaylorN{Float64}(9//10)
a isa TaylorN{Float64}
@test constant_term(a) == Float64(9//10)
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test that checks that an error (MethodError) is thrown if you do not make explicit the "numtype" T of the TaylorN{T} when using this constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for this addition (and sorry for the time to send my review). In general, LGTM, though I have included a comment requesting to add another tests. Aside from that, once that is implemented, I'm happy to merge it.

Thanks for taking a look! I added your suggestion, so this is now ready for merging.

lbenet
lbenet approved these changes Apr 17, 2024
View reviewed changes
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.

Thanks a lot @PerezHz! PR approved, and I'll merge it.

@lbenet lbenet merged commit 3a7360f into JuliaDiff:master Apr 17, 2024
12 checks passed
PerezHz referenced this pull request Apr 23, 2024
@PerezHz
Add methods in support of PerezHz/TaylorIntegration.jl#192 (#356)
fdfa666 
* Add RecursiveArrayTools extension and setindex! dispatch for mixtures

* Add RecursiveArrayTools extension and a Base.setindex! for mixtures

* Fix in setindex!

* Fix test

* Add RAT compat entry (found by Aqua)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbenet lbenet lbenet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PerezHz @coveralls @lbenet