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

Cannot understand the API design of Generic Math INumber.Create[XXXX] methods #62296

Closed
Tracked by #63548
maxild opened this issue Dec 2, 2021 · 10 comments
Closed
Tracked by #63548
Assignees
Milestone

Comments

@maxild
Copy link

maxild commented Dec 2, 2021

It seems like the TSelf TSelf.Create<TOther>(TOther value) (and friends) are what we can call ConvertFrom (in TypeConverter lingo). And there are no equivalent of TOther TSelf.ConvertTo(TSelf value).

So if I code up new number type

public readonly partial struct Fraction : IComparable<Fraction>, IEquatable<Fraction>, ISpanFormattable, IComparable
if FEATURE_GENERIC_MATH
#pragma warning disable SA1001, CA2252 // SA1001: Comma positioning; CA2252: Preview Features
        , IMinMaxValue<Fraction>
        , ISignedNumber<Fraction>
#pragma warning restore SA1001, CA2252
#endif // FEATURE_GENERIC_MATH
{
    // impl not important
}

I will implement the Create static method (and friends) (supporting whatever other numeric type I choose). But if I need to use this Fraction type somewhere else generically (in places where other number types can take its place, aka polymorphic programming), and need something like the following old hacky generic operators helper/util class

public static class Arithmetic
{
    // I cannot modify 'int Int32.Create(TOther value)', and
    // therefore I cannot convert a Fraction to an Int32 value
    // We need both 'TSelf TSelf.ConvertFrom(TOther value)' and TOther 'TSelf.ConvertTo(TSelf value)'
    public static int ToInt32<T>(T value) where T : struct
    {
        if (typeof(T) == typeof(decimal))
        {
            return (int)(object)((int)decimal.Floor((decimal)(object)value));
        }
        if (typeof(T) == typeof(Fraction))
        {
            return (int)(object)(Fraction.Floor((Fraction)(object)value).ToInt32());
        }
        throw new NotSupportedException("Type is not supported");
    }

    // This can be done via 'Fraction Fraction.Create(TOther value)'
    public static T FromInt32<T>(int value) where T : struct
    {
        if (typeof(T) == typeof(decimal))
        {
            return (T)(object)(new decimal(value));
        }
        if (typeof(T) == typeof(Fraction))
        {
            return (T)(object)(new Fraction(value));
        }
        throw new NotSupportedException("Type is not supported");
    }
}

And want to translate this Arithmetic helper into the new generic math preview feature in .NET 6

public static class Arithmetic
{
    // I made up the ConvertTo static polymorphic method (that does not exist on INumber<TSelf>)
    public static int ToInt32<T>(T value) where T : INumber<T> => T.ConvertTo<int>(value);

    public static T FromInt32<T>(int value) where T : INumber<T> => T.Create(value);
}

but of course this does NOT compile.

How can I solve this using the new generic math interfaces, when I cannot change the Int32.Create (ConvertFrom) code in the BCL?

We need something like TOther TSelf.INumber<T>.ConvertTo(TSelf value), that I can support on my own (non BCL) number type.

I think the conversion part of INumber have to be revisited.

@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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Dec 2, 2021
@ghost
Copy link

ghost commented Dec 3, 2021

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

Issue Details

It seems like the TSelf TSelf.Create<TOther>(TOther value) (and friends) are what we can call ConvertFrom (in TypeConverter lingo). And there are no equivalent of TOther TSelf.ConvertTo(TSelf value).

So if I code up new number type

public readonly partial struct Fraction : IComparable<Fraction>, IEquatable<Fraction>, ISpanFormattable, IComparable
if FEATURE_GENERIC_MATH
#pragma warning disable SA1001, CA2252 // SA1001: Comma positioning; CA2252: Preview Features
        , IMinMaxValue<Fraction>
        , ISignedNumber<Fraction>
#pragma warning restore SA1001, CA2252
#endif // FEATURE_GENERIC_MATH
{
    // impl not important
}

I will implement the Create static method (and friends) (supporting whatever other numeric type I choose). But if I need to use this Fraction type somewhere else generically (in places where other number types can take its place, aka polymorphic programming), and need something like the following old hacky generic operators helper/util class

public static class Arithmetic
{
    // I cannot modify 'int Int32.Create(TOther value)', and
    // therefore I cannot convert a Fraction to an Int32 value
    // We need both 'TSelf TSelf.ConvertFrom(TOther value)' and TOther 'TSelf.ConvertTo(TSelf value)'
    public static int ToInt32<T>(T value) where T : struct
    {
        if (typeof(T) == typeof(decimal))
        {
            return (int)(object)((int)decimal.Floor((decimal)(object)value));
        }
        if (typeof(T) == typeof(Fraction))
        {
            return (int)(object)(Fraction.Floor((Fraction)(object)value).ToInt32());
        }
        throw new NotSupportedException("Type is not supported");
    }

    // This can be done via 'Fraction Fraction.Create(TOther value)'
    public static T FromInt32<T>(int value) where T : struct
    {
        if (typeof(T) == typeof(decimal))
        {
            return (T)(object)(new decimal(value));
        }
        if (typeof(T) == typeof(Fraction))
        {
            return (T)(object)(new Fraction(value));
        }
        throw new NotSupportedException("Type is not supported");
    }
}

And want to translate this Arithmetic helper into the new generic math preview feature in .NET 6

public static class Arithmetic
{
    // I made up the ConvertTo static polymorphic method (that does not exist on INumber<TSelf>)
    public static int ToInt32<T>(T value) where T : INumber<T> => T.ConvertTo<int>(value);

    public static T FromInt32<T>(int value) where T : INumber<T> => T.Create(value);
}

but of course this does NOT compile.

How can I solve this using the new generic math interfaces, when I cannot change the Int32.Create (ConvertFrom) code in the BCL?

We need something like TOther TSelf.INumber<T>.ConvertTo(TSelf value), that I can support on my own (non BCL) number type.

I think the conversion part of INumber have to be revisited.

Author: maxild
Assignees: -
Labels:

area-System.Numerics, untriaged

Milestone: -

@jeffhandley jeffhandley added this to the 7.0.0 milestone Jan 9, 2022
@jeffhandley jeffhandley removed the untriaged New issue has not been triaged by the area owner label Jan 9, 2022
@jeffhandley
Copy link
Member

This is related to the PR feedback that was received when these interfaces were introduced: #54650 (comment)

@uecasm
Copy link

uecasm commented May 5, 2022

+1 from me, this interface just seems like it's repeating the mistakes of the Convert class by being a closed ecosystem with no way to inject custom class conversions.

If I invent a new FancyNumber class, there needs to be some way to express that FancyNumber can be generically converted from specific other INumber<> types (if that is indeed intended). As currently implemented, Create<TOther> is not that method -- while FancyNumber's own Create can convert to the primitive number types, the primitive numbers cannot convert to a FancyNumber.

Conversion from any type to any other should be supported whenever at least one type knows a direct conversion in one direction or the other -- so converting between FancyNumber and Decimal in either direction should work provided that FancyNumber itself can supply both conversions (as Decimal can't do so). Similar to the logic for converting operators, it should not permit chaining two such conversions via a third common intermediate type (unless the user is being explicit about invoking that conversion) -- so e.g. a FancyNumber couldn't be directly converted to a NoFrillsNumber even if they both know how to convert to Decimal and back, unless at least one of them explicitly converts to the other, or the user code explicitly invokes the intermediate conversion themselves.

One option might be to introduce the opposite direction TOther ConvertTo<TOther>(TSelf value) (or perhaps use the existing IConvertible interface for that purpose; that has its own caveats but at least it does have ToType), and have both Create and ConvertTo always implemented such that they try their opposite if they can't figure out how to do the conversion themselves, although care would be needed to avoid getting caught in infinite recursion.

Alternatively, it could (after optionally doing any fast-path conversions desired) fall back on using a TypeDescriptor/TypeConverter-based conversion and/or look for conversion operators. Either of these seem attractive both because conversion operators already implement the desired semantics and because both have a way to express both conversion directions, or to express that only a subset of conversions are valid -- it might be nice if the compiler or static analysis tools could figure out that a FancyNumber doesn't support conversion to byte (but might still support converting from it).

Either way, though, it's not useful if these methods only work for a limited set of built-in types.


On a tangentially-related note, I found it surprising when looking at the implementation of e.g. Decimal.Create that it appears to cast the value via object and back. Although on further reflection I can see why it does that, due to language limitations. But isn't that an entirely unnecessary boxing pessimization? Wouldn't it be better to have some kind of pattern-matching cast that avoids that (though a standard pattern-matching is won't work since you're wanting to match on a target type rather than a source type)? Otherwise perhaps Create just shouldn't exist at all and some TypeConverter based method (with reverse-direction fallback) should be the norm, since that inherently boxes too.

@tannergooding
Copy link
Member

class by being a closed ecosystem with no way to inject custom class conversions.

Any setup that is provided directly on the INumber class has potential pits of failures. There is a balance between providing something that is easy to use for the default scenario while still covering the extensibility required in more complex libraries.

The closest thing to having something that only requires INumber is to have a static bool TryConvertTo<TOther>(TSelf value, out TOther result) and bool TryConvertFrom<TOther>(TOther value, out TSelf result) where TSelf Create<TOther>(TOther value) effectively does:

if (TryConvertFrom(value, out var result))
{
    return result;
}

if (TOther.TryConvertTo(value, out var result))
{
    return result;
}

throw Exception();

These 3 methods would have a strict requirement that TSelf.TryConvertFrom and TOther.TryConvertTo be "self-contained" and not defer to any other TryConvertFrom or TryConvertTo methods and with Create itself effectively not being implementable by the user. This gives the "broadest" coverage without introducing cycles, but still leaves a pit of failure and a large gap where some types have more burden then others on providing conversions.

Either way, though, it's not useful if these methods only work for a limited set of built-in types.

Yes, and the intent is not for them to only be limited to these types. The primitive types are simple and should be directly supported by everything.

Slightly more generally, there are APIs on IBinaryInteger for getting the value as a two's complement span and on IFloatingPoint for getting the radix, significand, and exponent (also as two's complement spans). While not necessarily as "fast" as possible, this provides some reasonably performant fallback that allows most number kinds to be handled.

However, even with all of this there is still the issue of two libraries without dependencies on eachother and the only solution for this is to allow some user provided converter via a INumberConverter interface. There is no solution for this otherwise.

I found it surprising when looking at the implementation of e.g. Decimal.Create that it appears to cast the value via object and back.

This pattern does not actually box. .NET specializes value type generics and so all of the irrelevant paths are optimized away as dead code. Likewise the pattern of (SomeType)(object)value for a value that is actually a SomeType is handled by the JIT and the box/unbox is removed entirely. For reference types, boxing is already a "nop", although the code is not specialized in that scenario.

@uecasm
Copy link

uecasm commented May 5, 2022

These 3 methods would have a strict requirement that TSelf.TryConvertFrom and TOther.TryConvertTo be "self-contained" and not defer to any other TryConvertFrom or TryConvertTo methods and with Create itself effectively not being implementable by the user. This gives the "broadest" coverage without introducing cycles, but still leaves a pit of failure and a large gap where some types have more burden then others on providing conversions.

I agree with all of that. But I still think that this would be an improvement over the current implementation.

However, even with all of this there is still the issue of two libraries without dependencies on eachother and the only solution for this is to allow some user provided converter via a INumberConverter interface. There is no solution for this otherwise.

Yes, as you said, at least one of the types must know about the other type and agree to the conversion, or it must be the user that explicitly invokes conversion via an intermediate type that both know about (or potentially some longer chain thereof, which is riskier, but at the user's choice). It absolutely must not try to use a third type to convert on its own (not even string).

Having a user-supplied policy interface is not an option I had considered, and I'm not sure how feasible it would be in practice to get it passed around to the places it would need to go, nor how useful it would be in practice. By "user code" I was just meaning the code actually calling Create, which has the option to chain two different Create calls together via an intermediate type if it so wishes.

This pattern does not actually box.

Good to know.

@uecasm
Copy link

uecasm commented May 5, 2022

Actually now I have an even more basic confusion. Let's say I have a TValue : INumber<TValue> and TValue value. I want to convert this to a double.

  • (double) value does not work -- cannot cast TValue to double.
  • double.Create(value) does not work -- cannot access explicit implementation of INumber<double>.Create.
  • ((INumber<double>) double).Create(value) does not work -- cannot access static method in non-static context.
  • Convert<double>(value) does work, provided the following method is additionally defined:
    • private TOther Convert<TOther>(TValue value) where TOther : INumber<TOther> => TOther.Create(value);

This seems quite messy, but there doesn't appear to be any other way to get there... is there some reason that these are hidden? Or that there doesn't seem to be any syntax for calling explicit static methods?

(I also ran into this problem, but I think that's straying a bit off-topic.)

@jeffhandley
Copy link
Member

@tannergooding has this issue been addressed with the latest API changes?

@tannergooding
Copy link
Member

is there some reason that these are hidden? Or that there doesn't seem to be any syntax for calling explicit static methods?

The methods primarily exist to support usage from a generic context. If you have or know a concrete type, then you should use those directly.

@tannergooding has this issue been addressed with the latest API changes?

It should be, yes. Any additional issues or concerns should be tracked in a new issue.

@uecasm
Copy link

uecasm commented Jun 8, 2022

with the latest API changes?

Can you provide a link with more info on these changes?

The methods primarily exist to support usage from a generic context. If you have or know a concrete type, then you should use those directly.

There is still a generic context. I want to convert from a generic number to a non-generic number. There does not appear to be any way to do so except by writing and calling a separate two-generic-type method -- it's not possible to call any kind of conversion function even from only a single-generic-type context.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants