Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move special functions to a separate module #19560

Closed
wants to merge 2 commits into from
Closed

Conversation

musm
Copy link
Contributor

@musm musm commented Dec 11, 2016

Would be good to separate the file to a separate folder like LinAlg to have for additional pure Julia implementations of math functions.

@yuyichao
Copy link
Contributor

Again, please update/ping original PR instead of opening new ones!

@musm musm force-pushed the math branch 2 times, most recently from 3691e25 to 98466be Compare December 12, 2016 08:25
@ViralBShah
Copy link
Member

Why do all this moving? For now, can we leave things as they are until there is a larger overhaul/reason to move things?

@simonbyrne
Copy link
Contributor

I can see the appeal: it does follow the pattern of other submodules whereby we put all the necessary files under 1 directory. Especially since special is already a directory.

Alternatively, there is #8536, but not much has happened on that front, and no concrete plan has emerged.

@simonbyrne simonbyrne added the maths Mathematical functions label Dec 12, 2016
@tkelman
Copy link
Contributor

tkelman commented Dec 12, 2016

There is a plan. It's #18795.

@simonbyrne
Copy link
Contributor

It my be fair to call it a vague plan, but it is far from concrete.

@tkelman
Copy link
Contributor

tkelman commented Dec 12, 2016

What's not concrete about it?

@musm
Copy link
Contributor Author

musm commented Dec 12, 2016

it's bothersome that replutil.jl depends on the filename and location: https://github.com/musm/julia/blob/98466be678d1b5ad53de3deda8601462e9231f39/base/replutil.jl#L217
this will need to decoupled if the modules are moving out of the main julia repo.

@tkelman
Copy link
Contributor

tkelman commented Dec 12, 2016

Special functions could be if someone implements it. I don't think the rest of math.jl should move as soon though as that's more widely used.

@tkelman
Copy link
Contributor

tkelman commented Dec 12, 2016

some of these file locations are also listed as exceptions in contrib/add_license_to_files.jl

@simonbyrne
Copy link
Contributor

In that case, why don't we move the special functions to a new module, so that they're ready to be split out?

@musm
Copy link
Contributor Author

musm commented Dec 14, 2016

What happens in the future for packages that need a function like erf ? Will they all need to import the SpecialFunctions package?

IMO it might be worthy to keep some special functions in base mlib such as erf and gamma because of their fundamental importance in engineering and stats, etc. This practice is not uncommon to other mlibs, since they typically guarantee a high quality implementation for such critical functions

Thoughts ?

@simonbyrne

@ViralBShah
Copy link
Member

Yes, we will still want some of these in base. Also, I don't see why we can't reorganize things in base until default packages happens, if there is a good reason to do them.

@tkelman
Copy link
Contributor

tkelman commented Dec 14, 2016

airy, bessel, and probably dawson would be less disruptive to move in an earlier phase than erf and gamma.

@simonbyrne
Copy link
Contributor

Agreed: lets move those first; others we can evaluate on a case-by-case basis.

I think the main steps would be (until it's clear what form the stdlib would take):

  • move special/bessel.jl to specialfunctions/SpecialFunctions.jl, which contains a module of the same name; can optionally split this out into smaller files
  • all other files under special get moved to under math as in this PR.

Does that seem reasonable?

@musm
Copy link
Contributor Author

musm commented Dec 14, 2016

IMO I would keep bessel, gamma, and erf in Math (which admittedly is most of the function in special) , and move the rest to SpecialFunctions.

In case, someone has not seen this: http://www.evanmiller.org/statistical-shortcomings-in-standard-math-libraries.html (just something to consider/keep in mind)

@simonbyrne
Copy link
Contributor

How is that different than your current PR?

@simonbyrne
Copy link
Contributor

simonbyrne commented Dec 14, 2016

Bessel & Airy functions have long been marked for removal (#8536): they are the messiest to call (see all the glue code, global buffer variables and various issues about Float32, Float64 and BigFloat), and the least used. Why would you want to keep them in Base?

@musm
Copy link
Contributor Author

musm commented Dec 14, 2016

I see. Seems a little arbitrary then to keep erf and gamma in Math then, no?
So the options are:

  1. Move all special functions to SpecialFunctions.jl
  2. Move only erf and gamma (pretty arbitrary, but w/e)
  3. Move none.

@simonbyrne
Copy link
Contributor

erf,erfc and gamma are defined in the C math library, so there is some precedence here. Furthermore, they're also single argument functions, and used in a lot of domains.

There is some discussion of which functions to be moved also in #4301: perhaps best to continue this there.

@musm musm force-pushed the math branch 8 times, most recently from 0c80732 to 7042b5d Compare December 27, 2016 18:36
@musm musm changed the title Move math related function to own folder Move special functions to a seperate module Dec 27, 2016
@musm musm mentioned this pull request Dec 27, 2016
@ViralBShah
Copy link
Member

ViralBShah commented Dec 27, 2016

It would be nice to get this merged in some shape for 0.6, even if we can't get 100% agreement on module names.

@musm
Copy link
Contributor Author

musm commented Dec 27, 2016

Yes perhaps we need better names for these. Maybe Math and SpecFun are not the most suggestive. Suggestions?

@musm musm force-pushed the math branch 4 times, most recently from a42f594 to 2836965 Compare January 10, 2017 15:29
base/sysimg.jl Outdated

# special math functions
include("special/special.jl")
importall .SpecFun
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SpecialFunctions ?

@musm
Copy link
Contributor Author

musm commented Jan 10, 2017

Ok rebased. Are these the names we want? Math for elementary functions and more core math related functions and SpecialFunctions for all the special functions? Suggestions welcome. We should try to finalize this now for #18795 .

@timholy timholy changed the title Move special functions to a seperate module Move special functions to a separate module Jan 15, 2017
@musm musm closed this Jan 17, 2017
@musm musm deleted the math branch January 17, 2017 22:32
@simonbyrne
Copy link
Contributor

Any reason you closed this? AFAIK we still wanted to do this.

@musm
Copy link
Contributor Author

musm commented Jan 26, 2017

It got a thumbs down and the comment with a couple thumbs up #19560 (comment)
doesn't seem like people want to do this now

@tkelman
Copy link
Contributor

tkelman commented Jan 26, 2017

The recent versions of this PR were better than where it started.

@musm musm restored the math branch January 26, 2017 20:50
@musm
Copy link
Contributor Author

musm commented Jan 26, 2017

K, I'll reopen and see if there is interest in getting this in.

@musm musm reopened this Jan 26, 2017
@musm musm force-pushed the math branch 3 times, most recently from db52bb4 to dfd8423 Compare January 26, 2017 21:01
@musm musm force-pushed the math branch 2 times, most recently from 56bc23d to 4d56e6e Compare February 4, 2017 02:13
@musm musm closed this Feb 7, 2017
@musm musm deleted the math branch April 17, 2017 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants