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

[API Proposal]: Generic math Assign operators #60536

Closed
Tracked by #63548
msedi opened this issue Oct 18, 2021 · 14 comments
Closed
Tracked by #63548

[API Proposal]: Generic math Assign operators #60536

msedi opened this issue Oct 18, 2021 · 14 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics

Comments

@msedi
Copy link

msedi commented Oct 18, 2021

Background and motivation

Currently C# implements the += the same way as a a regular +. Which is OK for simple value types. But if the objects are getting large an intermediate copy needs to be created, although the intention is to reuse the existing object. Assume you have a dataset A with 50GB and another B with 50GB, then the operations

var C = A +B;
var A += B;

are the same. First of all it would be benefial to allow += as a seperate operator. Additionally the new generic math ideas could also add AddAssign, etc. Originally the workaround was to implement this yourself, but when this is already under consideration it would be good to also take a look at this topic.

Interestingly Expression already has AddAssign, etc. But if honestly have never tested if there is a different code generation.

API Proposal

namespace System
{
    public interface IFloatingPoint<T> 
    {
        public T AddAssign(T item);
        public T SubtractAssign(T item);
        public T DivideAssign(T item);
        public T MulAssign(T item);
    }
}

API Usage

var A = new Volume(2048, 2048, 1000);
var B = new Volume(2048, 2048, 1000);

A.AddAssign(B);

// Of course it would be nice if this would also work.
A += B;

Alternative Designs

No response

Risks

One problem is that I already needed this implementations and did it myself. While this works for my purposes I found the problem with very long running operations (e.f. +) that there is not way to cancel the process (for example with a CancellationToken).

I assume that you don't want to do this, and honestly I would do neither in this case. I just wanted to mention this because I stumbled across this problem.

@msedi msedi added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 18, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 18, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Oct 18, 2021

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Currently C# implements the += the same way as a a regular +. Which is OK for simple value types. But if the objects are getting large an intermediate copy needs to be created, although the intention is to reuse the existing object. Assume you have a dataset A with 50GB and another B with 50GB, then the operations

var C = A +B;
var A += B;

are the same. First of all it would be benefial to allow += as a seperate operator. Additionally the new generic math ideas could also add AddAssign, etc. Originally the workaround was to implement this yourself, but when this is already under consideration it would be good to also take a look at this topic.

Interestingly Expression already has AddAssign, etc. But if honestly have never tested if there is a different code generation.

API Proposal

namespace System
{
    public interface IFloatingPoint<T> 
    {
        public T AddAssign(T item);
        public T SubtractAssign(T item);
        public T DivideAssign(T item);
        public T MulAssign(T item);
    }
}

API Usage

var A = new Volume(2048, 2048, 1000);
var B = new Volume(2048, 2048, 1000);

A.AddAssign(B);

// Of course it would be nice if this would also work.
A += B;

Alternative Designs

No response

Risks

One problem is that I already needed this implementations and did it myself. While this works for my purposes I found the problem with very long running operations (e.f. +) that there is not way to cancel the process (for example with a CancellationToken).

I assume that you don't want to do this, and honestly I would do neither in this case. I just wanted to mention this because I stumbled across this problem.

Author: msedi
Assignees: -
Labels:

api-suggestion, area-System.Numerics, untriaged

Milestone: -

@huoyaoyuan
Copy link
Member

This doesn't work as +=. Primitive types are immutable, and += is creating a new one and assigning to the variable.

@MichalPetryka
Copy link
Contributor

I feel like:

  1. This should be rather a general csharplang proposal
  2. The signature should rather be something like:
static void AddAssign(ref Value this, Value value);

@huoyaoyuan
Copy link
Member

But if the objects are getting large an intermediate copy needs to be created

This looks more like the pattern used by BitArray. It's just an "Add" method.

I'd expect most generic math codes would assume values are immutable and copy by value, same as the semantics of primitive types.

@DrkWzrd
Copy link

DrkWzrd commented Oct 18, 2021

int a=0, b=2;
a+=b; //I thought this was syntactic sugar and actually the "real" code is
a= a+b; //we create a copy always, an then we assign it to the initial variable

Am I wrong?
In the other hand, if you are "adding" objects of 50GB, I think an operator "+" it's not the right choice, because, maybe (only maybe) you're not "adding", you're "merging". Subtle, but IMHO critical difference.

@msedi
Copy link
Author

msedi commented Oct 18, 2021

@huoyaoyuan: > This doesn't work as +=. Primitive types are immutable, and += is creating a new one and assigning to the variable.

This is correct for primitive types or readonly structs (maybe structs in general) always return a copy (if it does not contain reference types). It gets more complicated with classes.

However for all other types I can change the behavior by implementing my own + operator. I agree that the + shall return a new instance, but I have no chance to change the behavior of +=.

@msedi
Copy link
Author

msedi commented Oct 18, 2021

@KieranDevvs > Why can't you just overload the operator, modify the ref

I'm not quite sure if I got it. The problem is that + and += are the same. I don't want to change the data when using +, I just want to change it with += (and that what's currently not possible).

This fact of + and += is not very obvious if you are dealing with smaller data. Mostly you don't care, but if the data is larger then normal.

@msedi
Copy link
Author

msedi commented Oct 18, 2021

@DrkWzrd > Am I wrong?

No, you are right ;-) Somehow you could argue it's a merge, but in the end we have many operations on the data including Log, Exp, +, - ,*, etc. everything that is available in regular Math.

Maybe you are thinking that data is a pixture Volume, but we are talking about physical measurements here (X-Ray attenuation data).

Currently I can help myself with this topic, I was just curious if this should be considered in the generic math implementations from @tannergooding .

@msedi
Copy link
Author

msedi commented Oct 18, 2021

@MichalPetryka

  1. This should be rather a general csharplang proposal

Yes, that would be a language request to have a possbility to override += other than +.

  1. The signature should rather be something like:

Yes, that's maybe the better way.

@tannergooding
Copy link
Member

My own view is that I don't know if there is enough evidence that introducing this is worthwhile.

From the framework design guideline side of things, structs/value types are typically meant to be immutable. This helps avoid a number of issues that otherwise occur around copying, multithreading, etc.

From the language side of things, += has always had explicit semantics that a += b is transformed into a = a + b and introducing some new behavior here if an operator is defined may be confusing.

The general benefit of operators is that it helps with readability and precedence, especially in long chains of operations (e.g. T.Add(T.Mul(a, b), c) is less readable then a * b + c). However, you typically don't see assignment operators in these chains because the mutation gets hidden, can complicate understanding exactly what happens, etc. and so a += b only gets the readability benefit, over T.AddAssign(ref a, b), in most practical usage.

======================================

Now, with all that being said, ECMA-335 does define and allow for op_AdditionAssignment and friends to exist. If there is buy-off from the language I could see these being used in "builder-like" types as part of an optimized pattern. Someone would need to convince the C# LDM that its worth supporting these operators and why. This would likely come down to showing real world readability improvements, practical examples of where it improves code when mutable types are in play, etc. Such a discussion (not issue) would be started on dotnet/csharplang

@msedi
Copy link
Author

msedi commented Oct 18, 2021

My own view is that I don't know if there is enough evidence that introducing this is worthwhile.

Yes, that's what I also suspected. I agree.

From the language side of things, += has always had explicit semantics that a += b is transformed into a = a + b and introducing some new behavior here if an operator is defined may be confusing.

True. But I would rather make this optional of needed. If no += operator is explicitly given, I would fall back to the current behavior, so it would not cause any problems since += was not available in the past.

The general benefit of operators is that it helps with readability and precedence, especially in long chains of operations (e.g. T.Add(T.Mul(a, b), c) is less readable then a * b + c). However, you typically don't see assignment operators in these chains because the mutation gets hidden, can complicate understanding exactly what happens, etc. and so a += b only gets the readability benefit, over T.AddAssign(ref a, b), in most practical usage.

Someone would need to convince the C# LDM that its worth supporting these operators and why. This would likely come down to showing real world readability improvements, practical examples of where it improves code when mutable types are in play, etc. Such a discussion (not issue) would be started on dotnet/csharplang

Thats true and I cannot agree more. I was just curious if it should be considered in your design.

From my point of view (regarding the large amount of image data) I'm using the operators to create expressions that I then resolve into one large call. So that (given the captial letters are large volume objects) can be written like this:

Volume R = alpha * A + (1 - alpha) * B;

where each math operation creates a VolumeExpression that combines all expressions first and then returns a final volume. This is for performance reason since each operation would create another intermediate volume that causes unnecessary memory consumption and multiple reiterations through the volume.

So in the end I have everything I need ;-)

@tannergooding
Copy link
Member

This is not going to be readily actionable on our end without language support.

While we could define AddAssign methods ourselves, this is not going to be a good experience in a number of cases and may cause perf issues in others.

@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jan 24, 2022
@AraHaan
Copy link
Member

AraHaan commented Jan 24, 2022

I would also like this, but like Tanner said implement it with language support so it does not tank performance.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics
Projects
None yet
Development

No branches or pull requests

6 participants