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

Remove rad2deg and deg2rad from Base #54144

Open
LilithHafner opened this issue Apr 18, 2024 · 9 comments
Open

Remove rad2deg and deg2rad from Base #54144

LilithHafner opened this issue Apr 18, 2024 · 9 comments
Labels
excision Removal of code from Base or the repository maths Mathematical functions
Milestone

Comments

@LilithHafner
Copy link
Member

Things that don't need to be in Base should not be in Base.

@LilithHafner LilithHafner added the excision Removal of code from Base or the repository label Apr 18, 2024
@LilithHafner LilithHafner added this to the Potential 2.0 milestone Apr 18, 2024
@nsajko nsajko added the maths Mathematical functions label Apr 19, 2024
@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Apr 19, 2024

I agree, but why single those out? What about sin, cos, floating-point in general... Array (as opposed to Memory), OpenBLAS... so why single those out? They are small and the least of our problems?

@LilithHafner
Copy link
Member Author

sin/cos -> yes. They should be removed from Base, though it's a harder sell because more people use them.

floating-point in general -> no. No plausible, nontrivial program doesn't depend on floating point addition/multiplication. It's probably not worth the effort to excise it. If floating point math becomes less pervasive or approaches to basic floating point arithmetic become more diverse then maybe reconsider.

Array -> yes, though that does require changing the default return type of collect, list comprehensions, etc to Memory.

OpenBLAS -> absolutely!

so why single those out?

I single these two out because they are easy.

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Apr 19, 2024

I don't think we can drop them, annoying people (until 2.0), breaking change but neither should we expand it. @aplavin where else can that go in a package? I though/still think those are trivial functions on just Float64 (and smaller, and BigFloat...?! that I want to get rid of anyway). They are hopefully not costly in the sysimage, I might want them out o if it, just not Base. {Floats were just an impossible example, just not needed by Base itself, except possibly few divisions? Unlike rad2deg and deg2rad, sin/cos are actually non-trivial, unless done with assembly instructions, but I believe not done that way because of accuracy; also neither on matrices, I want LinearAlgebra out too, NOT needed for Julia itself... I just think we should start with the big stuff.]

I'm all for getting this all out and the small stuff with ENV var (not yet by default), but my suggestion was closed:
#50807

@aplavin
Copy link
Contributor

aplavin commented Apr 19, 2024

@aplavin where else can that go in a package?

Not sure, what exactly are you asking about?

As for dropping anything that Base doesn't need itself: yeah, would be nice, but with consistency. I don't see a fundamental difference between deg2rad, sin, sind, sincos in this regard. And also stuff like LogRange (it's going to be included into Base soon). And many more stuff, potentially.

@barucden
Copy link
Contributor

Is the proposal to move rad2deg/deg2rad from Base to another package (like it happened with, e.g., the functions that are now in SpecialFunctions.jl, or the whole DelimitedFiles.jl), or to remove them without providing an alternative implementation elsewhere?

I would be fine implementing rad2deg/deg2rad myself (after all, Base provides an obvious implementation). On the contrary, trigonometric functions are non-trivial, and IMHO should be provided from "official" sources (if not from Base, then from a package under JuliaMath).

@LilithHafner
Copy link
Member Author

Almost all proposed excisions are Base => package, not Base => nowhere. We definitely don't expect or want users to roll their own trig implementations.

This case could go either way, but I lean toward putting them in a library.

@andrewjradcliffe
Copy link
Contributor

A separate package just for two functions seems absurd. When prioritizing removal of items from Base, start with the largest items.

Moreover, code which resides in Base can be construed to be core functionality of a language. Unsurprisingly, Rust includes equivalents: f64::to_degrees, f64::to_radians.


Or, another way to take on it:

If we apply the proposed logic, then abs2, abs, nor, nand, etc. -- a whole world of "easy" functions disappear from Base. For such a radical change, serious motivation must be found.

@LilithHafner
Copy link
Member Author

A separate package just for two functions seems absurd

Agreed. This would live with sin, cos, etc in e.g. Math

"easy" functions

These functions are surprisingly nontrivial (e.g. #45947)

@andrewjradcliffe
Copy link
Contributor

"easy" was from:

I single these two out because they are easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
excision Removal of code from Base or the repository maths Mathematical functions
Projects
None yet
Development

No branches or pull requests

6 participants