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

revert broadening of arithmetic Any methods #45463

Closed
StefanKarpinski opened this issue May 26, 2022 · 11 comments · Fixed by #45489
Closed

revert broadening of arithmetic Any methods #45463

StefanKarpinski opened this issue May 26, 2022 · 11 comments · Fixed by #45489
Labels
kind:minor change Marginal behavior change acceptable for a minor release

Comments

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented May 26, 2022

PR #45320 removed a couple of ancient recent and dicey overly generic methods for arithmetic operators: +(x) = x and *(x) = x. But it left equally generic fallbacks like -(x) = Int8(-1)*x and -(x, y) = x + (-y). I would argue that these should also require subtyping Number to opt into them. This is potentially a breaking change, but it seems weird to expect these things to work if your type isn't a number (and most other numbery things don't work). I would argue that the policy should be: if you don't subtype Number then you don't get any arithmetic fallbacks, you only get fallbacks that truly make sense for all objects.

Btw, several times people have insisted on specification of what the "interface for Number" is. There is no interface for Number, it is not an interface—there are no requirements that must be satisfied by a Number. What Number is, is a way to opt into a bunch of generic behaviors like these.

@StefanKarpinski StefanKarpinski added the kind:minor change Marginal behavior change acceptable for a minor release label May 26, 2022
@KristofferC
Copy link
Sponsor Member

KristofferC commented May 26, 2022

Not very ancient, #44564 :). The point was to make them generic (ref #43903)

@StefanKarpinski
Copy link
Sponsor Member Author

Uy, why did we let that happen? On the other hand, I'm glad it's not that ancient since it means we can change it.

@dkarrasch
Copy link
Member

It happened because of the discussion in #43903 which was largely in favor of making the methods generic (I was (one of) the most skeptical ones). Then in #44564 I was begging for feedback, and received one approval and no strong objections, just some weak objections from @vtjnash.

What Number is, is a way to opt into a bunch of generic behaviors like these.

Are you suggesting to resolve #43903 by subtyping Number? From a mathematical perspective that sounds wild, but types are not necessarily meant to model 1-1 mathematical categories.

@N5N3
Copy link
Member

N5N3 commented May 27, 2022

I agree that * and + have their mathmatic meanings.
But user can also use them as a syntactic sugar (like 'a' * 'a').
As for #43903, user can define their AbstractVecSpace if they want these fallback.
(It's also strange that -(::Any) assumes its input has * defined.)

@oscardssmith
Copy link
Member

Do we want to add a Group type in between Number and Any?

@AriMKatz
Copy link

AriMKatz commented May 27, 2022

Ideally a vector space (or group) trait: https://google-research.github.io/dex-lang/prelude.html#s-13

@DilumAluthge
Copy link
Member

DilumAluthge commented May 27, 2022

It happened because of the discussion in #43903 which was largely in favor of making the methods generic (I was (one of) the most skeptical ones). Then in #44564 I was begging for feedback, and received one approval and no strong objections, just some weak objections from @vtjnash.

I strongly objected to #44564. I had already posted my objection in #43903 (comment), and I had assumed I didn't need to re-post in #44564.

@DilumAluthge
Copy link
Member

Anyway, I would like us to revert #44564.

(To prevent merge conflicts, we need to revert #44564 and #45320 in the same PR.)

@StefanKarpinski
Copy link
Sponsor Member Author

Are you suggesting to resolve #43903 by subtyping Number? From a mathematical perspective that sounds wild, but types are not necessarily meant to model 1-1 mathematical categories.

I don't think that only Number can have these behaviors; we could define these for Number and AbstractArray for example. Or even AbstractArray{<:Number}.

@StefanKarpinski StefanKarpinski changed the title remove remaining arithmetic Any methods revert broadening of arithmetic Any methods May 27, 2022
@StefanKarpinski
Copy link
Sponsor Member Author

No harm done here, @dkarrasch, I'd much rather you make forward progress on things and we occasionally have to revert something that not being able to get things done.

@DilumAluthge
Copy link
Member

If we really wanted to, we could define a new abstract type in Base of the form abstract type VectorSpaceElement end. And then these fallback methods would be defined for Union{Number, AbstractArray{<:Number}, VectorSpaceElement}. For example:

-(x::Union{Number, AbstractArray{<:Number}, VectorSpaceElement}) = Int8(-1)*x

Then, if you want to opt-in to these fallback methods for your custom type, and you don't want your custom type to be a subtype of Number or AbstractArray, you could make your custom type a subtype of VectorSpaceElement.

Now, with that all being said, it's not clear to me why this VectorSpaceElement abstract type needs to live in Base. Wouldn't it be better for it to live in a package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants