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

Reconfigure rationals #2796

Merged
merged 2 commits into from
May 21, 2024
Merged

Reconfigure rationals #2796

merged 2 commits into from
May 21, 2024

Conversation

kddnewton
Copy link
Collaborator

@kddnewton kddnewton commented May 8, 2024

This eliminates the subnode on RationalNode and replaces it with two integer fields, which represent the ratio for the rational. It also reduces those two integers if they both fit into 32 bits.

Importantly, this PR does not implement bignum reduction. That's something I'd like to consider for the future, but it's simple enough for now to leave them unreduced, which makes it more useful than it used to be.

Fixes #2796.

@kddnewton kddnewton changed the title Split rationals Reconfigure rationals May 21, 2024
This eliminates the subnode on RationalNode and replaces it with two
integer fields, which represent the ratio for the rational. It also
reduces those two integers if they both fit into 32 bits.

Importantly, this PR does not implement bignum reduction. That's something
I'd like to consider for the future, but it's simple enough for now to
leave them unreduced, which makes it more useful than it used to be.
@kddnewton kddnewton merged commit fe7d6c6 into main May 21, 2024
56 of 57 checks passed
@kddnewton kddnewton deleted the rationals branch May 21, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant