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

Introduce new numerics #200

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

@ponylang-main ponylang-main added status - new The RFC is new and ready for discussion. discuss during sync Should be discussed during an upcoming sync labels Feb 28, 2022
@SeanTAllen
Copy link
Member

Is there a reason to tie these different packages together? Would there be something we lose by discussing each individually?

text/0000-expand-math.md Outdated Show resolved Hide resolved
text/0000-expand-math.md Outdated Show resolved Hide resolved
Co-authored-by: Joe Eli McIlvain <joe.eli.mac@gmail.com>
@rhagenson
Copy link
Member Author

Is there a reason to tie these different packages together? Would there be something we lose by discussing each individually?

I expect that you mean to suggest creation of discrete RFCs. As I answered for the "primary goals" comment for clarification, this RFC is meant to "expand" math is two ways. The first is to expand by providing structure, the second by providing functionality. If we want to make this RFC only the former, then I open RFCs for each subpackage that is fine by me. I thought opening with the initial additions as discussed (a long time ago) in Zulip was "better" than breaking it up without any discussion as to how we want to move towards a more expanded math package.

@SeanTAllen
Copy link
Member

@rhagenson if these aren't tried to one another, I think that an RFC for each would be good.

I would focus on adding support for things like Complex numbers, Rational numbers, and those because the design issues for each of those is worthy of individual RFCs.

I'm not even sure that Rational numbers should go in math. Math to be is more about "algos and processes" like "matrix multiplication" etc and not so much about things like complex or rational numbers. I'd seriously consider having rational numbers be their own top-level package. Ditto complex.

And fairly reasonable argument could be made for Arbitrary precision numbers to go into builtin. I think we would decide not to, but the argument could reasonably be made based on consistency of F32 and friends being there.

@rhagenson
Copy link
Member Author

I am not against breaking this RFC into pieces, but wanted to present all the initial pieces at once. I am personally in favor of the addition functionality discussed here being outside of builtin since it will rely on builtin, but builtin will not rely on any of the suggested additions.

text/0000-expand-math.md Outdated Show resolved Hide resolved
@rhagenson
Copy link
Member Author

rhagenson commented Mar 1, 2022

During Sync on 2022-03-01 I wrote down the following notes for how to improve this RFC:

  • Add implementation details; for example, what does a Rational look like internally?
  • Add motivation for having canonical implementation for these types; addressing having interoperability of numerics across the Pony ecosystem
  • Mention intention to be compliant with existing numeric traits such as Real, Integer, etc
  • As an external reference, Haskell was noted as having a strong numerics hierarchy

@rhagenson
Copy link
Member Author

rhagenson commented Mar 4, 2022

@SeanTAllen Would me switching this to a draft PR prevent the discuss during sync label from being reapplied? I have doubts that I will get all the updates done before next Sync in time for folks to review the changes. I have no issue with more reviews/comments/discussion, but want to prevent issues of this RFC continually reappearing for Sync with only small or in-progress edits.

My current plan is:

  • Add top-down mermaid graph for the numeric type hierarchy as we have it today
  • Add top-down mermaid graph for the numeric type hierarchy with these new types included
  • Fix the typing errors for at least Rational (I have a comment in the example implementation in the most recent commit)

@SeanTAllen
Copy link
Member

SeanTAllen commented Mar 4, 2022

Would me switching this to a draft PR prevent the discuss during sync label from being reapplied?

No.

See https://github.com/ponylang/rfcs/blob/main/.github/workflows/add-discuss-during-sync.yml#L3 for events that cause the label to be added.

@rhagenson
Copy link
Member Author

rhagenson commented Mar 6, 2022

  • Add BigFloat numeric to mermaid chart
  • Add Complex parameterized on Real
  • Add Rational parameterized on Integer
  • Discuss breaking this RFC into smaller dedicated RFCs concerning
    • Adding numeric types to builtin
    • Introducing new math functionality as suggested in this RFC (e.g., having a Constant primitive)

@rhagenson
Copy link
Member Author

rhagenson commented Mar 8, 2022

On Sync 2022-03-08, we discussed breaking this RFC into smaller dedicated RFCs with this RFC/PR specifically becoming a "Introduce new numerics; Rational, BigInt, BigFloat, Complex" and splitting the other content off to better allow us to discuss the merits of each addition/change.

@SeanTAllen
Copy link
Member

I'm in favor of breaking this apart into different RFCs.

@rhagenson rhagenson changed the title Expand math package Introduce new numerics Mar 14, 2022
@rhagenson rhagenson closed this Mar 14, 2022
@rhagenson rhagenson deleted the expand-math branch March 14, 2022 00:10
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Mar 14, 2022
@rhagenson rhagenson restored the expand-math branch March 14, 2022 00:10
@rhagenson rhagenson reopened this Mar 14, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 14, 2022
@rhagenson
Copy link
Member Author

Apologies about temporarily closing this -- I tried changing the name of the expand-math branch to add-numerics and I guess that is not a thing I can do without forcing this PR to close.


1. restructure the `math` package into distinct subpackages; allowing for separation of concerns over continuing to build a monolithic `math` package
2. provide common `math` data types; for example, `BigInt`, `Rational`, and `Complex`
I propose we add the aforementioned numeric types into `builtin` so they exist alongside the other standard numeric types. These introduced numeric types **must** existing within the current numeric type hierarchy by being compliant with existing numeric traits.
Copy link
Member

Choose a reason for hiding this comment

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

For me, the preference would be to keep builtin as minimal as possible, so I'd prefer to put these new types in a new package (or series of packages), unless there is a strong motivation for it to go in builtin.

Pretty much all of the existing public types in builtin have hard requirements forcing them to be there, with a reason like one of the following:

  • they are used by core language constructs (None, string literals, numeric literals, etc)
  • they need to use raw pointers (Array, String, etc)
  • they use compile_intrinsic for one or more function definitions
  • they are part of the Env, which is passed to the Main actor on entry
  • they are used by one of the types that meet the above criteria

I don't think these new types have any hard requirements forcing them to go in builtin, so I believe they shouldn't be there.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can discuss this further on Sync and I will update the RFC according to our conversation. I have no particular need for these to be added to builtin so no objection to an agreed upon other location such as a newly created numerics or adding these to math (as was the RFC state prior to my latest changes).

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Mar 15, 2022
@rhagenson
Copy link
Member Author

rhagenson commented Mar 15, 2022

Notes from Sync 2022-03-15:

  • Update Unresolved Questions w/ details -- this RFC is the high-level overview of how the types fit into the numeric hierarchy, but discrete RFC for each numeric is needed providing complete implementation details
  • Move suggestion out of builtin into new numerics package

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 15, 2022
new val min_value()
new val max_value()
...
```
Copy link

@jasoncarr0 jasoncarr0 Mar 16, 2022

Choose a reason for hiding this comment

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

I understand that this has already been present, but given that this is re-design I'm not clear on the benefit of these constructors, but there's some obvious costs. Is there a use case for max_value() / min_value() that makes sense for all numbers? Given that this doesn't work for BigInt/BigFloat/similar types I'd be hesitant to place it at the top of the hierarchy.

Can we instead make a Bounded interface? This will only break code which relies on these methods being present in any Real[A]

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what you are suggesting. What's a "bounded interface"?

Copy link
Member

Choose a reason for hiding this comment

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

I think he means a new interface whose name is Bounded, which has the min and max value methods on it.

Copy link
Member

Choose a reason for hiding this comment

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

@jasoncarr0 is that what you meant? re: Bounded interface? if yes, did you specifically mean structural typing or were you using "interface" more loosely?

Choose a reason for hiding this comment

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

I meant a literal Pony interface, that is:
These constructors would be part of an interface named Bounded, while the other methods would be part of this trait.

Choose a reason for hiding this comment

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

Actually there is also an argument for removing Ordered, but only because this trait is really the lowest possible. Mostly that the implementations for things like matrices would be sketch but making them numerics makes sense and is powerful.

fun op_and(y: A): A => this and y
fun op_or(y: A): A => this or y
fun op_xor(y: A): A => this xor y
fun op_not(): A => not this

Choose a reason for hiding this comment

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

This can't be implemented by a BigInt, so it's odd that a BigInt can't actually be an Integer

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I don't understand your comment. Can you try explaining in a different way?

Copy link
Member

Choose a reason for hiding this comment

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

You can't do a bitwise "not" operation on a BigInt - at least not by the normal semantics of a bitwise "not" operation.

You can't because the you'd expect that any bits "more significant" than the most significant bit of the value would be set to 1.

But a BigInt has no upper bound, and thus there is no limit to the number of leading 1 bits implied by inverting its bits

Copy link
Member

Choose a reason for hiding this comment

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

I still don't understand @jemc.

"This can't be implemented by a BigInt, so it's odd that a BigInt can't actually be an Integer"

Can you explain how what you said is related to Jason's comment?

I don't understand why not being able to be implemented BigInt makes it odd that BigInt can't actually be an Integer.
I don't understand what is being said here.

Copy link
Member

Choose a reason for hiding this comment

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

I'll copy Jason's words and insert parentheticals with my own commentary:

This (the op_not function) can't be implemented by a BigInt (for the reason mentioned in my previous comment)

So it's odd that a BigInt (which is conceptually a kind of integer) can't actually be an Integer (it cannot be a subtype of the Integer trait, because the Integer trait includes the op_not function).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jemc. I understand now.

fun bswap(): A
```

`Integer` will exist above `BigInt` and as such would require defining the above methods -- however `BigInt` will be defined via other numerics so these methods could be applied recursively.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what this means. What are "the above methods"? Those listed on the Integer trait?

"BigInt will be defined via other numerics so these methods could be applied recursively" <-- I don't understand what this means either. There seems to be something that I am supposed to understand about "other numerics" that BigInt will be defined via (did I miss that somewhere, if yes, bringing information forward so that you don't have to hold large chunks of RFC in your head would be good). I'm also not sure what "applied recursively" means in this context.

fun atanh(): A
```

`FloatingPoint` will exist above `BigFloat` and as such would require defining the above methods -- many of which are ill-defined under arbitrary precision, or are functions using C-FFI and/or LLVM intrinsics.
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused. Why would we do it this way if it has these problems? This seems like an argument against doing it this way. I feel like there's some meaning here that I am missing.

@SeanTAllen
Copy link
Member

I find this RFC very confusing as written. I don't understand what large sections are attempting to convey. I think I'm going to need someone to walk me through it as at the moment, I'm not really sure how to even phrase my "I don't understand" statements.

@SeanTAllen
Copy link
Member

I'd like to see this broken apart further into an RFC for where Rational and Complex fit with the existing numerics hierarchy and a separate discussion of the best way to implement "Big" numerics. There are rather complicated issues that appear to apply to only "the bigs" that I think will take some time to resolve and during that time we can probably agree to how Rational and Complex will fit in with the existing, do detailed RFCS for each and implement them before the questions around "the bigs" are resolved.

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Mar 22, 2022
@adrianboyko
Copy link

Any thoughts on the possibility of having builtin complex primitive types? I think the RFC is proposing that all complex values will be object instances, which seems like it would be troublesome in various use-cases. For instance, I'm working on signal processing in Pony and have an actor that produces a stream of Vector[U8C]s from RTL-SDR hardware. I'm currently defining type U8C is U16, which gives me a complex type that matches C's representation of the same, so I can use C libraries that do vector processing on complex values.

C11 standard says:

Each complex type has the same representation and alignment requirements as an array type containing exactly two elements of the corresponding real type; the first element is equal to the real part, and the second element to the imaginary part, of the complex number.

libvolk (C library for vector operations) defines the following:

typedef char complex lv_8sc_t
typedef short complex lv_16sc_t
typedef long complex lv_32sc_t
typedef long long complex lv_64sc_t
typedef float complex lv_32fc_t
typedef double complex lv_64fc_t

In Pony, I imagine types like:

primitive I8C is SignedIntegerComplex[I8C]
primitive I16C is SignedIntegerComplex[I16C]
primitive I32C is SignedIntegerComplex[I32C]
primitive I64C is SignedIntegerComplex[I64C]
primitive I128C is SignedIntegerComplex[I128C]
primitive U8C is UnsignedIntegerComplex[U8C]
primitive U16C is UnsignedIntegerComplex[U16C]
primitive U32C is UnsignedIntegerComplex[U32C]
primitive U64C is UnsignedIntegerComplex[U64C]
primitive U128C is UnsignedIntegerComplex[U128C]
primitive F32C is FloatingPointComplex[F32C]
primitive F64C is FloatingPointComplex[F64C]

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 6, 2022
@rhagenson
Copy link
Member Author

@adrianboyko Since I am the author on this RFC, I will chime in.

Any thoughts on the possibility of having builtin complex primitive types?

I am not against it outright because I do not know what all that would entail. However, this RFC will be refined (when I have time) to propose the higher level aspects of what numerics we wish to introduce. Which will then be followed by specific RFCs for the structure and implementation of those specific numerics. That is to say, how complex numerics are implemented is critical to their successful adoption and definitely needs to be a part of the RFC on their implementation.

@jemc
Copy link
Member

jemc commented Jun 14, 2022

The idea of primitive complex types was discussed a bit on the sync call.

The advantage of not having an extra object allocation is a great one.

To make that idea work, they would need to be in the builtin package and would likely need many of their methods to be implemented as compiler_intrinsic. This would be possible, but in my opinion it's not ideal.

In my opinion, this would be a good use case for struct-like value types, which I have discussed a bit in the past (and implemented in the Savi compiler for prototyping). The basic premise is that it would be a new kind of Pony type declaration that is declared just like a class, but can only have let fields (no var fields) - and at runtime it behaves like a tuple (no object allocation, unless it gets "boxed" into an abstract type).

Having a "struct-like value type" like this would allow for avoiding the extra object allocation for the complex numeric types, as well as anything similar that others wanted to build in user-defined libraries. It also would solve the "newtype" use case discussed in #65. It would also help use cases where performance-sensitive code is currently using tuples for performance reasons where they would otherwise prefer to use classes.

@jemc jemc removed the discuss during sync Should be discussed during an upcoming sync label Jun 14, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 14, 2022
@jemc jemc removed the discuss during sync Should be discussed during an upcoming sync label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status - new The RFC is new and ready for discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants