giordano / PolynomialRoots.jl

Fast complex polynomial root finder, with support for arbitrary precision calculations
Other
54 stars 15 forks source link

Performance improvements #24

Open nsajko opened 2 years ago

nsajko commented 2 years ago

Prevents run-time dispatch in the most basic PolynomialRoots.roots method.

giordano commented 1 year ago

Do you have any benchmarks? Tests are failing, I'd be OK with requiring a newer version of Julia if that helps, but tests are failing also on nightly (haven't checked why)

nsajko commented 1 year ago

The failing tests are for roots5, which shouldn't be changed at all by this (remaining) commit.

I can do some benchmarking if you want. JET still shows the same warning on Julia nightly, though, so this change still fixes the run time dispatch.

nsajko commented 1 year ago

Regarding the tests that fail for Julia 1.0: the reason seems to be the begin indexing syntax.

The begin syntax also causes an error on Julia 1.4.2, but 1.5.4 seems to work fine.

giordano commented 1 year ago

but tests are failing also on nightly (haven't checked why)

Tests are failing on nightly independently of this PR. It looks like the result of https://github.com/giordano/PolynomialRoots.jl/blob/ccc22225212c484d6093da51ecdca5abea5185e9/test/runtests.jl#L163-L164 and https://github.com/giordano/PolynomialRoots.jl/blob/ccc22225212c484d6093da51ecdca5abea5185e9/test/runtests.jl#L182-L183 changed so that they go off the previously set absolute tolerance.