-
Notifications
You must be signed in to change notification settings - Fork 143
8338529: Initial iteration of numerics modeling interfaces #1917
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
Conversation
|
👋 Welcome back darcy! A progress list of the required criteria for merging this PR into |
|
@jddarcy This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
| * @param op1 the first operand | ||
| * @param op2 the second operand | ||
| */ | ||
| OC min(OC op1, OC op2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a default method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if min/max should be included in the interface. I think it depends what role any non-operator methods have in interfaces intended for witnesses.
However, as long as the methods are here, they could be default methods, yes.
To acknowledge some feedback received on a different channel, if the OC type variable were required to extend Comparable, then all the lessThan, greaterThan, etc. method could have default methods written in terms of the results of compareTo. However, if we want to move away from Comparable, then extended its usage here probably isn't a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if min/max should be included in the interface. I think it depends what role any non-operator methods have in interfaces intended for witnesses.
However, as long as the methods are here, they could be default methods, yes.
To acknowledge some feedback received on a different channel, if the OC type variable were required to extend Comparable, then all the lessThan, greaterThan, etc. method could have default methods written in terms of the results of compareTo. However, if we want to move away from Comparable, then extended its usage here probably isn't a good idea.
PS Pushed another changeset refactoring to have less than be the base operation and using default methods for the rest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure what the intended design rules are here, but for anything algebraic, fewer axioms are much better than more (murkily related) axioms. Having many abstract interface points is the same as many axioms — the witnesses each have their own story for each API point independently.
The default method approach is very wise, and the body of the default should be in an implnote in the docs. It amounts to a proof that shows how to (e.g.) deduce min and max from lower-level axioms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure what the intended design rules are here, but for anything algebraic, fewer axioms are much better than more (murkily related) axioms. Having many abstract interface points is the same as many axioms — the witnesses each have their own story for each API point independently.
The default method approach is very wise, and the body of the default should be in an implnote in the docs. It amounts to a proof that shows how to (e.g.) deduce min and max from lower-level axioms.
To be precise, for a default method in an interface (unless various special conditions hold*), there should be an implSpec tag (not implNote) that at least documents the general logic and calls to other methods of the interface used in the default method's body. Replicating the full body of the method in the implSpec tag can be acceptable, especially if it is a one-liner, but it is not requited in general. We have multiple examples of this kind of using the JDK APIs, including in javax.lang.model where I'm one of the maintainers.
The implSpec tags I included in the changeset to Orderable for it to have default methods follow the general conventions of tag usage for that situation in the JDK.
Thanks and HTH.
(* The special conditions where an implSpec tag on a default method can be skipped include cases like a default method of a sealed interface where there will be no implementations outside of the shared maintenance domain of the interface.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure what the intended design rules are here, but for anything algebraic, fewer axioms are much better than more (murkily related) axioms. Having many abstract interface points is the same as many axioms — the witnesses each have their own story for each API point independently.
The default method approach is very wise, and the body of the default should be in an implnote in the docs. It amounts to a proof that shows how to (e.g.) deduce min and max from lower-level axioms.
One of the implicit design questions here is "are these the interfaces that allow operator overloading?" or "are these the interfaces that declare various algebraic properties?"
Since many of the numerical types of possible interest are closer to, say, floating-point-like types that have few algebraic properties rather than integer-like types that have more, I don't want to preclude floating-point-like types from participating in operator overloading because of their lack of strong algebraic properties. Matrices/vectors would also fall closer to floating-point-like rather than integer-like and I would not want to preclude matrices/vectors from benefiting from operators either.
I included a slide in my 2025 JVMLS talk on numerics speculating that a future refinement of these kinds of interfaces could include an idiom to indicate "yes, this type actually obeys the ring axioms" or "... the field axioms", etc., but that is not included in this early "lumpy" iteration.
I would fully expect some evolution of the set of interfaces, what methods go where, the set of interfaces, etc. as we can more experience using these trial types with type classes.
Thanks.
|
I would prefer to see the ordering part tolerating partial orders (NaNs), and then factor the ordering part on top of both ints and floats. Also, in the name of reducing primitives, I think a |
|
|
Mailing list message from Brian Goetz on valhalla-dev: Overall I agree with the direction that while we should be _inspired by_ I think we can borrow much hard-won experience from our friends in the Haskell also relegates the algebraic structure (ring, monoid, etc) to a The point about implementing both `Num` and `Ord` is one of those
-------------- next part -------------- |
| package java.lang; | ||
|
|
||
| /** | ||
| * Indicates a type supports the basic binary arithmetic operations of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Indicates a type supports the basic binary arithmetic operations of | |
| * Indicates a type that supports the basic binary arithmetic operations of |
|
From experience working on #1937, pushed another changeset to this PR discussion closure properties of different operations and how they are expected to be documented. In other words, if value is not returned for a particular combination of operands, the expectation is that an ArithmeticException will be thrown and the conditions under which that exception would be thrown should be documented in the numerical type. |
To make progress on getting experience using type classes, I'm going to push the current state of the PR. Reconsidering how ordering is handled be done as future work, along with other refinements of the modeling interfaces. Thanks. |
|
/integrate |
|
Going to push as commit 7a8dc9c. |
A refinement of some earlier ideas on the numerics modeling interfaces to inform further discussions.
This is a "lumpy" rather than "splitty" design in terms of favoring a smaller number of interfaces with more functionality rather than a larger number of interfaces making smaller distinctions.
Various design comments and to-do's noted in the code.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1917/head:pull/1917$ git checkout pull/1917Update a local copy of the PR:
$ git checkout pull/1917$ git pull https://git.openjdk.org/valhalla.git pull/1917/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1917View PR using the GUI difftool:
$ git pr show -t 1917Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1917.diff
Using Webrev
Link to Webrev Comment