TaylorSeries.jl icon indicating copy to clipboard operation
TaylorSeries.jl copied to clipboard

Make `index_table` a Vector{Vector{UInt128}}, and add tests

Open lbenet opened this issue 7 years ago • 13 comments

This is a small modification addressing [#85]. It simply carries on the hashing of the vectors of integers in UInt128 arithmetic.

lbenet avatar Feb 19 '17 16:02 lbenet

This will slow down (I would guess by rather a lot) every single calculation with the package. It will also have its own upper bound on the number of possible variables.

I think a better solution is the one I posted in https://github.com/JuliaDiff/TaylorSeries.jl/issues/85#issuecomment-280924935

although that will require more work to make everything type stable again.

dpsanders avatar Feb 19 '17 17:02 dpsanders

Coverage Status

Coverage remained the same at 97.147% when pulling 6e274706ea12a652c0d898f20c5fcafac89a918c on index_table into 99a4d8ebc0d5a0186d65c49d85ab11d7c6dbfc03 on master.

coveralls avatar Feb 19 '17 17:02 coveralls

This will slow down (I would guess by rather a lot) every single calculation with the package. It will also have its own upper bound on the number of possible variables.

Both statements are correct. Running perf/fateman.jl with Julia v0.5 yields almost a factor ~1.7 slower if #86 would be merged, though (interestingly) no more allocations. (Using Int128 instead of UInt128 yields essentially the same results.)

I think a better solution is the one I posted in #85 (comment) although that will require more work to make everything type stable again.

I also agree with this. Yet, this is quite tricky, since the choice must depend on the values.

A possible approach could be to define a (global) parameter which fixes the parameterization of generate_tables{T} and in_base{T}, which eventually defines the type of index_table. Throwing an error message would also be nice, but I'm not sure how to do that.

lbenet avatar Feb 19 '17 21:02 lbenet

@PerezHz The performance hit is important, and as @dpsanders states, it is not common to have situations where the used hashed numbers is so important. I suggest that you use the branch "index_table" to get access to what has been done here.

lbenet avatar Feb 20 '17 02:02 lbenet

I think we should move away from any kind of global table. Then each Taylor polynomial can store a reference to an object of the relevant type / size.

dpsanders avatar Feb 20 '17 02:02 dpsanders

I agree that we should look for alternatives (including moving away from the global table scheme), but if such alternatives involve memory overhead, I don't think that's the way to go.

lbenet avatar Feb 20 '17 02:02 lbenet

I closed this, push -f the corresponding commits adapted to the new organization of the files (#87), and keep this open, in case it may be needed.

lbenet avatar Mar 04 '17 18:03 lbenet

Coverage Status

Coverage remained the same at 97.579% when pulling e1c624bb7e6c73358d21dcd9bdff42c6ed41f159 on index_table into 0a9184229bb344cad23b21069aa2c9cea672ed0c on master.

coveralls avatar Mar 04 '17 18:03 coveralls

Coverage Status

Coverage remained the same at 97.579% when pulling e1c624bb7e6c73358d21dcd9bdff42c6ed41f159 on index_table into 0a9184229bb344cad23b21069aa2c9cea672ed0c on master.

coveralls avatar Mar 04 '17 18:03 coveralls

Thanks @lbenet and @dpsanders for the work you are putting into this; it is helping me a lot! Perhaps a note should be added in the documentation regarding the fact that TaylorN works with at most 65 variables and if more are needed then the current solution is to use the index_table branch?

PerezHz avatar Mar 05 '17 19:03 PerezHz

Coverage Status

Coverage remained the same at 98.251% when pulling b7966f6f7a336c02a960c1652614b22f505ba22d on index_table into 88f2fe976bd02b49d2e9f96318c6c3189b78994f on master.

coveralls avatar Mar 07 '17 14:03 coveralls

Coverage Status

Coverage remained the same at 98.274% when pulling 3a4d4a4ee489c53e2c21c8cc224974a72148def2 on index_table into 4a40685deacca2a3e5d7b3024c175cee0af07058 on master.

coveralls avatar Mar 08 '17 04:03 coveralls

Coverage Status

Coverage decreased (-1.5%) to 96.552% when pulling 8766c5871efa9a3a24b424906699ed6ba324e105 on index_table into d4f0c20a4db83f84782daf617ef0944ce3673301 on master.

coveralls avatar Jun 10 '17 01:06 coveralls

Addressed by #314 and #316

lbenet avatar Feb 04 '24 03:02 lbenet