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

Preliminary work on static analogs for Taylor1 #241

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

mewilhel
Copy link

#240

  • Includes the definition of STaylor1 structure and constructors.
  • Adds the following functions for the STaylor1 type: +, -. *, exp, zero, one, evaluate, ==, functionality to iterate and get indices.

I've added a small benchmarking library for individual operators. This was mostly to provide a means to confirm that none of the operators with STaylor1 objects are allocating.

This doesn't include promotion for different types such as +(STaylor{N,T}, S) or a number of other operators. I figured it was probably better to get some feedback before going too much further.

Copy link
Contributor

@dpsanders dpsanders 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 a great start, thanks!

Haven't had a chance to look all the way through but I commented on a few things that I think can be simplified using broadcasting.

q = STaylor1(r)
q2 = STaylor1(r)
y = rand()
S["Taylor1{$i,Float64} + Float64"] = @benchmarkable f(+, $x, $y, $n)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to use metaprogramming to script these tests.

@@ -31,11 +33,23 @@ end
for T in (:Taylor1, :HomogeneousPolynomial, :TaylorN)
@eval iszero(a::$T) = iszero(a.coeffs)
end
function iszero(a::STaylor1)
Copy link
Contributor

Choose a reason for hiding this comment

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

all(iszero, a.coeffs) perhaps?

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@@ -205,6 +219,48 @@ end
-(a::Taylor1{T}, b::TaylorN{S}) where {T<:NumberNotSeries,S<:NumberNotSeries} =
-(promote(a,b)...)

@inline +(a::STaylor1{N,T}, b::STaylor1{N,T}) where {N, T<:Number} = STaylor1(add_tuples(a.coeffs, b.coeffs))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just use STaylor1(a.coeffs .+ b.coeffs)?

Copy link
Author

Choose a reason for hiding this comment

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

Done!



function *(a::STaylor1{N,T}, b::T) where {N, T<:Number}
STaylor1{N,T}(scale_tuple(a.coeffs, b))
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, should just be a.coeffs .* b

Copy link
Author

Choose a reason for hiding this comment

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

Done!



#=
Tuple operations taken from ForwardDiff.jl
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all of these can be replaced by simple broadcasts these days.

src/evaluate.jl Outdated
@@ -30,18 +31,41 @@ function evaluate(a::Taylor1{T}, dx::S) where {T<:Number, S<:Number}
end
evaluate(a::Taylor1{T}) where {T<:Number} = a[0]

function evaluate(a::STaylor1{N,T}, dx::T) where {N, T<:Number}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe some functions like this can be defined only once for both Taylor1 and STaylor1.
Is there an AbstractTaylor1 type?

Copy link
Author

Choose a reason for hiding this comment

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

I've looked into this a little bit. I haven't added an AbstractTaylor1 type yet.

We could do this for the evaluate(a::Taylor1{T}) function (just have both functions call get_order) but I haven't been able to figure out a way to avoid this for most other functions.

@coveralls
Copy link

coveralls commented Apr 10, 2020

Coverage Status

Coverage decreased (-3.7%) to 91.597% when pulling ed558cd on mewilhel:master into 234491b on JuliaDiff:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.1%) to 93.14% when pulling 66fa8ce on mewilhel:master into 234491b on JuliaDiff:master.

@dpsanders
Copy link
Contributor

Check here for an alternative approach to many of these functions:

https://github.com/dpsanders/StaticTaylorSeries.jl/blob/203f79543688a4c32e29434c0da945aea9ce4614/src/StaticTaylorSeries.jl#L70

using the ntuple function.
If this is as performant then I think it's preferable.

@dpsanders
Copy link
Contributor

Sorry, this fell under the radar.

I personally think that the separate StaticTaylorSeries.jl package would be a more suitable place for this.

Are you still working in this direction?

@mewilhel
Copy link
Author

@dpsanders No worries. I had to divert my attention elsewhere for a quite while. The separate package idea works for me.

I'm currently throwing a separate package together and I should have the preliminary work done by the end of this week.

@dpsanders
Copy link
Contributor

@mewilhel I was meaning this package that I wrote quite a while ago that has a lot of overlapping functionality:

https://github.com/dpsanders/StaticTaylorSeries.jl

@mewilhel
Copy link
Author

@dpsanders Ah! Sorry, I haven't looked at this in a while. That sounds good too. I'll fork a copy of that repository and transfer some of the content there.

@mewilhel
Copy link
Author

@dpsanders I've created a pull request for StaticTaylorSeries.jl: dpsanders/StaticTaylorSeries.jl#2.

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.

3 participants