RFC: Remove special functions from Base #20427

ararslan · 2017-02-03T06:28:32Z

This PR removes bessel, hankel, airy, error, dawson, eta, zeta, digamma and its inverse, trigamma, and polygamma functions from Base as part of #19598. The dependency on openspecfun will remain until rem2pi can be written without calling into openspecfun.

Fixes #8536

JeffBezanson · 2017-02-03T06:45:52Z

base/exports.jl

-    bessely,
-    bessely0,
-    bessely1,
-    besselyx,
    beta,


Should we move beta and lbeta as well?

I thought about that, but those are simple extensions of gamma/lgamma, so in the interest of not doing too much at once, I left those for now.

JeffBezanson · 2017-02-03T06:47:49Z

Looks like libspecfun.a is accidentally checked in here.

ararslan · 2017-02-03T06:50:48Z

Ah whoops, thanks. Will fix.

tkelman · 2017-02-04T08:15:07Z

Why do we need to check in openspecfun source, couldn't we leave those functions ccalls as-is until there are pure Julia replacements? I don't see much advantage in moving a subset of the C code into this repository when the build machinery for openspecfun already exists and is pretty well-proven by now.

tkelman · 2017-02-04T08:18:21Z

src/Makefile

@@ -203,12 +203,18 @@ $(BUILDDIR)/support/libsupport.a: $(SRCDIR)/support/*.h $(SRCDIR)/support/*.c
 $(BUILDDIR)/support/libsupport-debug.a: $(SRCDIR)/support/*.h $(SRCDIR)/support/*.c
 	$(MAKE) -C $(SRCDIR)/support debug BUILDDIR='$(abspath $(BUILDDIR)/support)'

+$(BUILDDIR)/specfun/libspecfun.a: $(SRCDIR)/specfun/*.h $(SRCDIR)/specfun/*.c $(BUILDDIR)/support/libsupport.a


does this actually depend on libsupport?

then the makefile rule shouldn't be written as if it does

ararslan · 2017-02-04T08:23:38Z

Why do we need to check in openspecfun source, couldn't we leave those functions ccalls as-is until there are pure Julia replacements? I don't see much advantage in moving a subset of the C code into this repository when the build machinery for openspecfun already exists and is pretty well-proven by now.

Fair points. The advantage of moving just rem2pi stuff into libjulia is to remove the dependency on openspecfun, since we'd then only be using it for one function, which seems silly. But I'm fine reverting all of that and just leaving openspecfun as-is if that's what's best.

oxinabox · 2017-02-04T08:26:49Z

(See also simonbyrne/libm.jl for a mostly complete implementation of the requisite functionality in Julia.)

Do you mean @simonbyrne 's 2 years stale repo, or one of the more recent works? (which have also involved @simonbyrne )

tkelman · 2017-02-04T08:32:02Z

Moving C code around and getting the build system to be happy with it can be a fair amount of work, especially if the hopefully-not-too-distant plan is to replace it anyway? Openspecfun will be good to get rid of particularly since it's a Fortran library (so if it gets moved to a package where your plan is to compile it from source, you can expect a lot of users to not have a Fortran compiler installed), but it sounds like we're not all the way there yet.

ararslan · 2017-02-04T18:35:23Z

@tkelman I have indeed put quite a bit of work into getting it right, and the build system is happy with it in my local tests (current CI failure is because I forgot to remove the docs, so doctests are failing), but I understand the concern.

Yes, my plan for the SpecialFunctions package is for users to compile from source unless:

They're on macOS, in which case there's a Homebrew formula for openspecfun with bottles, or
They're on Windows, in which case I'm hoping we can cross-compile and host some binaries on BinTray.

I think gfortran is still included in most Linux distros, so I'm not too worried about Linux users being left in the cold. Even if it isn't, I would venture a guess that most Linux users could figure out how to install gfortran and compile.

@oxinabox Yes, I meant Simon's libm.jl, which includes a mostly complete implementation of the only function we need here. It's only missing Payne-Hanek reduction for large arguments. If another package has a complete implementation, that's great.

StefanKarpinski · 2017-02-06T20:52:37Z

I'm also fine with continuing to build and ship libopenspecfun with Julia and just moving the Julia code that uses that library into a package, which would allow rem2pi to continue using libopenspecfun. That's similar to the libRmath situation we had for a long time. Hopefully this will take a bit less time to sort out, but there's no need to get to the perfect state here.

ararslan · 2017-02-06T20:56:10Z

@StefanKarpinski Yeah, Tony and I chatted about that yesterday. I think that's a good course of action, especially since SpecialFunctions.jl can then continue to use libopenspecfun without having to use BinDeps for 0.6.

StefanKarpinski · 2017-02-06T22:13:50Z

Great. That's a plan then.

ararslan · 2017-02-07T08:19:38Z

Things are good to go on this end, and the package will (hopefully) soon be registered! See JuliaLang/METADATA.jl#7859. Once that's done, this should be good to merge.

tkelman · 2017-02-07T22:46:46Z

base/deprecated.jl

@@ -672,65 +661,6 @@ end
 # Deprecate isimag (#19947).
 @deprecate isimag(z::Number) iszero(real(z))

-@deprecate airy(z::Number) airyai(z)
-@deprecate airyx(z::Number) airyaix(z)
-@deprecate airyprime(z::Number) airyaiprime(z)


these deprecations are fairly new and should stay

How should I deprecate them if the functions they should be deprecated in favor of are no longer in Base? Something like

function airy(z::Number) error("airy(z) has been deprecated in favor of airyai(z) in the SpecialFunctions package.") end

?

Oh, but I guess the deprecation targets are still exported from Base but themselves emit errors on use

tkelman · 2017-02-07T22:47:14Z

base/deprecated.jl

-        :erfcx, :erfi, :dawson,
-        # base/special/bessel.jl
-        :airyai, :airyaiprime, :airybi, :airybiprime,
-        :besselj0, :besselj1, :bessely0, :bessely1,


does the package defined deprecated vectorized versions of these?

No, didn't seem necessary but I can add them to the package if you think that'd be best.

we haven't had a release with them deprecated, it's only been on master, so I would do that for the sake of anyone upgrading

ararslan · 2017-02-09T05:00:02Z

Per a chat with Tony, all deprecations have been moved from here to the package and symbols already deprecated here are just added to the list here that emits an error pointing to SpecialFunctions. The functions are properly deprecated in SpecialFunctions.

tkelman · 2017-02-09T19:26:06Z

base/special/gamma.jl

-        signflip(m, zeta(s,z) * (-gamma(s)))
-    end
-end
-
 # TODO: better way to do this


doesn't look like these are needed any more

tkelman · 2017-02-09T19:27:00Z

base/special/gamma.jl

-    # the coefficients here are Float64(bernoulli[2:9])
-    ψ += t*w * @evalpoly(w,0.16666666666666666,-0.03333333333333333,0.023809523809523808,-0.03333333333333333,0.07575757575757576,-0.2531135531135531,1.1666666666666667,-7.092156862745098)
-end
-
 signflip(m::Number, z) = (-1+0im)^m * z
 signflip(m::Integer, z) = iseven(m) ? z : -z


only used in code that was moved?

Yep, you're right (here and above). Will remove. Thanks!

oh but since you were importing the Base version in the package, you'll want to add these there instead

Right, that's what I figured I'd do.

tkelman · 2017-02-09T19:40:43Z

I might miss having erf and erfc without an import a little bit, but LGTM aside from the dead code noted above, and the post-merge TODO (JuliaLang/METADATA.jl#7859 (comment)) on the package importing for Julia versions after this merges (and up to the deprecations getting deleted, so should also verify isdefined before extending anything from Base).

We should move the PR by @oxinabox over to the package as well.

ararslan · 2017-02-10T04:51:32Z

32-bit AppVeyor failure seems unrelated (an error in pkg tests). Log backed up here. Worth restarting AV?

StefanKarpinski · 2017-02-10T05:10:00Z

I don't think so, that package cloning problem happens occasionally due to bad connectivity from AppVeyor machines. The failure is after all the rest of the tests passed and the package tests passed on the other Windows build, so should be fine. As long as the Travis builds are good, you're in the clear.

stevengj · 2017-02-10T13:10:46Z

@ararslan, could I have a collaborator invite in SpecialFunctions?

simonbyrne · 2017-02-10T16:01:40Z

@stevengj I sent you an invite

ararslan · 2017-02-10T19:18:34Z

@stevengj You've had a pending JuliaMath membership invitation for months now. 😉

ararslan · 2017-02-11T01:29:13Z

I'd like to get this in before something else creates a conflict and I have to rebase, causing the PR to languish in the Travis OS X queue for an eternity, so speak now or forever hold your peace. 😄

ararslan added deprecation This change introduces or involves a deprecation maths Mathematical functions labels Feb 3, 2017

ararslan mentioned this pull request Feb 3, 2017

things we should deprecate, 0.6 edition #19598

Closed

22 tasks

ararslan requested review from simonbyrne and tkelman February 3, 2017 06:30

JeffBezanson reviewed Feb 3, 2017

View reviewed changes

ararslan force-pushed the aa/farewell-specialfuncs branch from 4c7a618 to ca7eeb1 Compare February 3, 2017 07:02

tkelman reviewed Feb 4, 2017

View reviewed changes

ararslan force-pushed the aa/farewell-specialfuncs branch from f04da2f to 00a6cda Compare February 6, 2017 23:19

tkelman reviewed Feb 7, 2017

View reviewed changes

ararslan force-pushed the aa/farewell-specialfuncs branch from 00a6cda to 3e8fb7a Compare February 9, 2017 04:57

ararslan force-pushed the aa/farewell-specialfuncs branch from 3e8fb7a to 6213414 Compare February 9, 2017 17:57

tkelman added this to the 0.6.0 milestone Feb 9, 2017

tkelman reviewed Feb 9, 2017

View reviewed changes

tkelman mentioned this pull request Feb 9, 2017

Improve the Type consistency of Special Functions (Fixes #17474) #18584

Closed

ararslan force-pushed the aa/farewell-specialfuncs branch 2 times, most recently from 65ee782 to ff974d2 Compare February 10, 2017 03:44

Remove some special functions from Base

29d012c

ararslan force-pushed the aa/farewell-specialfuncs branch from ff974d2 to 29d012c Compare February 10, 2017 19:41

ararslan changed the title ~~WIP: Remove special functions from Base~~ RFC: Remove special functions from Base Feb 10, 2017

ararslan merged commit e355685 into master Feb 11, 2017

ararslan deleted the aa/farewell-specialfuncs branch February 11, 2017 06:32

oxinabox mentioned this pull request Feb 11, 2017

Future of openlibm in julia #18102

Closed

giordano mentioned this pull request Feb 17, 2017

Mention in NEWS.md the removal of special functions #20642

Merged

carlomontec mentioned this pull request Jun 26, 2017

Make test fails at compiling rem2pi when using icc/ifort+MKL #22551

Closed

RFC: Remove special functions from Base #20427

RFC: Remove special functions from Base #20427

Conversation

ararslan commented Feb 3, 2017 • edited Loading

JeffBezanson Feb 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Feb 3, 2017

ararslan commented Feb 3, 2017

tkelman commented Feb 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Feb 4, 2017

oxinabox commented Feb 4, 2017

tkelman commented Feb 4, 2017

ararslan commented Feb 4, 2017

StefanKarpinski commented Feb 6, 2017

ararslan commented Feb 6, 2017

StefanKarpinski commented Feb 6, 2017

ararslan commented Feb 7, 2017 • edited Loading

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

ararslan commented Feb 9, 2017

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

tkelman commented Feb 9, 2017

ararslan commented Feb 10, 2017

StefanKarpinski commented Feb 10, 2017

stevengj commented Feb 10, 2017

simonbyrne commented Feb 10, 2017

ararslan commented Feb 10, 2017

ararslan commented Feb 11, 2017

ararslan commented Feb 3, 2017 •

edited

Loading

JeffBezanson Feb 3, 2017 •

edited

Loading

ararslan commented Feb 7, 2017 •

edited

Loading