Reorganize files #87

lbenet · 2017-03-02T19:09:51Z

This reorganizes the contents differently. In particular, it allows to exploit metaprogramming tools, though I have only done that to to a limited extent. My idea is to reorganize things and provide a more consistent use of the internal methods.

…s ... ... delete HomogeneousPolynomial(x), and adapt everything to this change.

…iation

coveralls · 2017-03-02T21:16:40Z

Coverage increased (+0.4%) to 97.579% when pulling bdc41ed on reorg_files into 99a4d8e on master.

coveralls · 2017-03-03T15:05:52Z

Coverage increased (+0.4%) to 97.561% when pulling 782317a on reorg_files into 99a4d8e on master.

`Taylor1` and `HomogeneousPolynomial` inner constructors use now `check_taylor1_order!()` and `check_taylorn_order!()` to modify the passed coefficients if needed for the construction.

dpsanders · 2017-03-03T20:37:42Z

I started commenting but think it will be easier for me to do a PR with some code simplification once this is merged.

I certainly agree with splitting up the huge files into pieces. Is this all that is done in the PR? It would be best to make a separate PR for any actual code changes.

dpsanders

I suggest you move these changed to a new PR.

dpsanders · 2017-03-03T20:32:16Z

src/arithmetic.jl

+    @eval begin
+        =={T<:Number,S<:Number}(a::$T{T}, b::$T{S}) = ==(promote(a,b)...)
+        function =={T<:Number}(a::$T{T}, b::$T{T})
+            a, b = fixorder(a, b)


I think the objective should be to get rid of fixorder.

fixorder is needed because you may be comparing two Taylor1 (or TaylorN) polynomials with different order.

But you are creating a whole new Taylor1 object just to check equality, where you could just be checking that the coeffs that match are equal, and the extra coeffs in the longer vector are all 0.

You are right; I'll see if I can do something to deal only with the coefficients.

dpsanders · 2017-03-03T20:32:32Z

src/arithmetic.jl

+        function =={T<:Number}(a::$T{T}, b::$T{T})
+            a, b = fixorder(a, b)
+            @simd for i in eachindex(a.coeffs)
+                @inbounds (a.coeffs[i] == b.coeffs[i]) || return false


Can't this whole loop be replaced by a.coeffs == b.coeffs?

Maybe, though I'm not sure if that would be faster or not.

I would guess it would be the same speed and much less writing. You are just writing out by hand testing equality of two vectors as far as I can tell.

If you don't use fixorder then testing equality would indeed be more complicated.

dpsanders · 2017-03-03T20:34:31Z

src/arithmetic.jl

+
+function iszero{T}(a::HomogeneousPolynomial{T})
+    @simd for i in eachindex(a.coeffs)
+        @inbounds (a.coeffs[i] == zero(T)) || return false


Replace loop by all(a.coeffs .== 0)

Thanks for the advice! I will test it now.

I just implemented this; tests are failing ...

The idea of using iszero was for optimizing in places like mul!; without such optimizations, we loose a bit of performance in perf/fateman.jl, around ~15%.

Sorry, I did something wrong regarding my check with your proposal; indeed tests pass. Regarding performance (perf/fateman.jl) it is slightly degraded (few percent), the number of allocations grows as well as the total memory used grows. So the current implementation, though not so nice, is a bit better.

dpsanders · 2017-03-03T20:35:46Z

src/arithmetic.jl

+
+## zero and one ##
+zero{T<:Number}(a::Taylor1{T}) = Taylor1(zero(a.coeffs[1]), a.order)
+one{T<:Number}(a::Taylor1{T}) = Taylor1(one(a.coeffs[1]), a.order)


Replace by Taylor1(one(T), a.order)?

This is necessary for mixed types, e.g. Taylor{TaylorN{T}}`.

julia v0.5> x, y = set_variables("x y") 2-element Array{TaylorSeries.TaylorN{Float64},1}: 1.0 x + 𝒪(‖x‖⁷) 1.0 y + 𝒪(‖x‖⁷) julia v0.5> t = Taylor1(x, 10) 1.0 x + 𝒪(‖x‖⁷) + 𝒪(t¹¹) julia v0.5> one(eltype(t)) 1.0 + 𝒪(‖x‖¹)

one(a.coeffs[1]) is the same as one(typeof(a.coeffs[1])) so my suggestion should always work.

lbenet · 2017-03-03T21:19:49Z

I just responded to some of your comments. I will test a replacement involving all that you point out somewhere.

Sorry about this PR, which includes some other changes beside reorganizing the files. Some things are related to use some metaprogramming to avoid repeating the same lines of code; other changes include SIMD optimizations, or using the new syntax for inner constructors.

Let me check your suggestion, and then it is a good idea to merge (otherwise, I'll continue putting things in top of this)...

lbenet · 2017-03-03T21:35:40Z

Regarding this suggestion, tests are failing in several places. So I will stop here making additions... Sorry, once again.

coveralls · 2017-03-03T22:11:16Z

Coverage increased (+0.4%) to 97.579% when pulling ebeb588 on reorg_files into 99a4d8e on master.

lbenet · 2017-03-03T23:13:09Z

@dpsanders One commit more 😄 : separate tests in several files and use Base.Test...

coveralls · 2017-03-04T00:32:58Z

Coverage decreased (-97.1%) to 0.0% when pulling 36d50f1 on reorg_files into 99a4d8e on master.

coveralls · 2017-03-04T03:31:02Z

Coverage increased (+0.4%) to 97.579% when pulling 7bcfaef on reorg_files into 99a4d8e on master.

coveralls · 2017-03-04T03:46:37Z

Coverage increased (+0.4%) to 97.579% when pulling cc0f5b2 on reorg_files into 99a4d8e on master.

lbenet · 2017-03-04T06:40:26Z

Indeed, Number includes Real and Complex, but also Taylor1 and the other types defined in TaylorSeries. The reason to restrict it is to consider only those cases.
EDIT: For example, to treat addition of a Taylor1 plus a RealOrComplex.

(Things like Taylor1{Taylor1{T}} should still be possible, but I didn't check this. That actually allows to construct polynomials in many variables in a tree form, but it is something I haven't had time to explore more.)

dpsanders

Since this is mostly reorganization, and tests pass, I think this can be merged.

lbenet · 2017-03-04T17:28:28Z

Ok. I'll merge this and address the fixorder and the issue about one in a different PR. Thanks for the review!

Luis Benet added 5 commits March 2, 2017 11:03

Reorganize files

f059a55

Shortcut for independent HomogeneousPolynomial{T} variables with test…

86016b6

…s ... ... delete HomogeneousPolynomial(x), and adapt everything to this change.

Clarify docstrings of set_variables adding jldoctest's

613a79c

Clean-up arithmetic.jl (metaprogramming) and add RealOrComplex abbrev…

cd9cd9f

…iation

Further metaprogramming and clean-up code in arithmetics.jl

bdc41ed

PerezHz mentioned this pull request Mar 3, 2017

Add jacobian! #88

Closed

Solve a type-unstability issue, and further organization of code

782317a

Luis Benet added 2 commits March 3, 2017 13:26

New inner type constructor syntax, and use of new auxiliary functions

62d8184

`Taylor1` and `HomogeneousPolynomial` inner constructors use now `check_taylor1_order!()` and `check_taylorn_order!()` to modify the passed coefficients if needed for the construction.

SIMD optimizations, use Base.findfirst instead of findfirst, with tests

ebeb588

dpsanders reviewed Mar 3, 2017

View reviewed changes

Separate tests in several files, and use Base.Test system

36d50f1

lbenet changed the title ~~WIP: Reorganize files~~ Reorganize files Mar 4, 2017

Delete @async which was left in test/runtests.jl

cc0f5b2

lbenet force-pushed the reorg_files branch from 7bcfaef to cc0f5b2 Compare March 4, 2017 03:19

dpsanders approved these changes Mar 4, 2017

View reviewed changes

lbenet merged commit 0a91842 into master Mar 4, 2017

lbenet deleted the reorg_files branch March 4, 2017 18:01

lbenet mentioned this pull request Mar 4, 2017

Make index_table a Vector{Vector{UInt128}}, and add tests #86

Closed

This was referenced Mar 4, 2017

Cannot use more than 65 variables with TaylorN #85

Closed

Issues with mixtures of Taylor1 and TaylorN #89

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize files #87

Reorganize files #87

lbenet commented Mar 2, 2017

coveralls commented Mar 2, 2017 •

edited

Loading

coveralls commented Mar 3, 2017 •

edited

Loading

dpsanders commented Mar 3, 2017

dpsanders left a comment

dpsanders Mar 3, 2017

lbenet Mar 3, 2017

dpsanders Mar 4, 2017

lbenet Mar 4, 2017

dpsanders Mar 3, 2017

lbenet Mar 3, 2017

dpsanders Mar 4, 2017

dpsanders Mar 3, 2017

lbenet Mar 3, 2017

lbenet Mar 3, 2017

lbenet Mar 3, 2017

lbenet Mar 3, 2017

dpsanders Mar 3, 2017

lbenet Mar 3, 2017

dpsanders Mar 4, 2017 •

edited

Loading

lbenet commented Mar 3, 2017

lbenet commented Mar 3, 2017

coveralls commented Mar 3, 2017 •

edited

Loading

lbenet commented Mar 3, 2017

coveralls commented Mar 4, 2017 •

edited

Loading

coveralls commented Mar 4, 2017 •

edited

Loading

coveralls commented Mar 4, 2017 •

edited

Loading

lbenet commented Mar 4, 2017 •

edited

Loading

dpsanders left a comment

lbenet commented Mar 4, 2017

Reorganize files #87

Reorganize files #87

Conversation

lbenet commented Mar 2, 2017

coveralls commented Mar 2, 2017 • edited Loading

coveralls commented Mar 3, 2017 • edited Loading

dpsanders commented Mar 3, 2017

dpsanders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpsanders Mar 4, 2017 • edited Loading

Choose a reason for hiding this comment

lbenet commented Mar 3, 2017

lbenet commented Mar 3, 2017

coveralls commented Mar 3, 2017 • edited Loading

lbenet commented Mar 3, 2017

coveralls commented Mar 4, 2017 • edited Loading

coveralls commented Mar 4, 2017 • edited Loading

coveralls commented Mar 4, 2017 • edited Loading

lbenet commented Mar 4, 2017 • edited Loading

dpsanders left a comment

Choose a reason for hiding this comment

lbenet commented Mar 4, 2017

coveralls commented Mar 2, 2017 •

edited

Loading

coveralls commented Mar 3, 2017 •

edited

Loading

dpsanders Mar 4, 2017 •

edited

Loading

coveralls commented Mar 3, 2017 •

edited

Loading

coveralls commented Mar 4, 2017 •

edited

Loading

coveralls commented Mar 4, 2017 •

edited

Loading

coveralls commented Mar 4, 2017 •

edited

Loading

lbenet commented Mar 4, 2017 •

edited

Loading