search

acroy / Expokit.jl

Julia implementation of EXPOKIT routines
Other
26 stars 11 forks source link

Upgrade to Julia 0.7/1.0 #34

Closed acroy closed 6 years ago

acroy commented 6 years ago

Addresses #33 : First I was trying to stay compatible to 0.6, but it was too much hassle. So with this PR support for versions < 0.7 is dropped.

garrison commented 6 years ago

Also note that there are a few warnings under CI:

┌ Warning: opnorm(LinearOp, Inf) is not defined, fall back to using `anorm = 1.0`.
│ To suppress this warning, please specify the anorm keyword manually.
└ @ Expokit ~/build/acroy/Expokit.jl/src/expmv.jl:8

EDIT: sorry, I had forgotten that this warning is intentionally generated by this package; carry on.

garrison commented 6 years ago

Regarding benchmarking, my (nonscientific) results indicate that the tests take ~70% longer to run on julia 0.7 (even with this pull request). I'm not even sure that the culprit is within Expokit.jl itself, but it would be nice to get to the bottom of this ... EDIT: perhaps related to https://github.com/JuliaLang/julia/issues/28432

acroy commented 6 years ago

I agree that we should do a comparison under 0.6 and 0.7/1.0. The question is if we should merge this anyways to get rid of deprecation warnings etc and open a new issue to track the possible regression?

mforets commented 6 years ago

I suggest that you merge and request to tag a new release, adding in the notes that this version drops support for v0.6; note that different versions can be benchmarked with 

julia> benchmarkpkg("Expokit", tagged-version-or-branch)