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

Move pi to trig and e to exp, fix atan2 #1012

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Move pi to trig and e to exp, fix atan2 #1012

merged 1 commit into from
Oct 11, 2023

Conversation

Izaakwltn
Copy link
Collaborator

There are some types that need pi or e but shouldn't need Complex or Polar in order to satisfy Elementary to get them.

This shouldn't break external code, since Elementary will be a marker class and will still contain them.

@stylewarning

I've also checked this against cl-quil/discrete and it doesn't seem to have broken anything.

library/math/elementary.lisp Outdated Show resolved Hide resolved
library/big-float/impl-default.lisp Outdated Show resolved Hide resolved
library/big-float/impl-default.lisp Outdated Show resolved Hide resolved
library/big-float/impl-sbcl.lisp Outdated Show resolved Hide resolved
library/math/dual.lisp Outdated Show resolved Hide resolved
@Izaakwltn Izaakwltn force-pushed the pi-e-liberation branch 4 times, most recently from 87d98ae to 2cffe14 Compare October 11, 2023 18:14
Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

Does this current rev actually type check? It seems like a type error to call BFConst on something that's not Unit -> Big-Float. See comments herein.

library/big-float/impl-default.lisp Outdated Show resolved Hide resolved
library/big-float/impl-sbcl.lisp Outdated Show resolved Hide resolved
library/big-float/impl-sbcl.lisp Outdated Show resolved Hide resolved
library/big-float/impl-default.lisp Outdated Show resolved Hide resolved
library/big-float/impl-default.lisp Outdated Show resolved Hide resolved
library/big-float/impl-sbcl.lisp Outdated Show resolved Hide resolved
@stylewarning
Copy link
Member

I'm concerned about why this program is type checking... BFConst :: (Unit -> Big-Float) -> Big-Float but you're providing it Big-Float in certain places. Are the tests passing because MPFR is being loaded (which doesn't have this same issue) and not the fallback?

@stylewarning
Copy link
Member

Sorry for the noise. The tests are failing for Allegro. You can try testing with :coalton-portable-bigfloat in your *features* before you compile Coalton.

@Izaakwltn Izaakwltn force-pushed the pi-e-liberation branch 3 times, most recently from e69ec3d to 896e930 Compare October 11, 2023 21:32
@Izaakwltn
Copy link
Collaborator Author

Tests are all passing now, and default Big-Float pi and ee are guaranteed to be necessary precision

Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

a few little things but I think they ought to be addressed for standard library cleanliness

library/big-float/impl-default.lisp Show resolved Hide resolved
library/big-float/impl-sbcl.lisp Outdated Show resolved Hide resolved
@Izaakwltn Izaakwltn force-pushed the pi-e-liberation branch 2 times, most recently from 80ce394 to 2405b94 Compare October 11, 2023 23:12
@stylewarning stylewarning merged commit 2c4d0bb into main Oct 11, 2023
17 checks passed
@stylewarning stylewarning deleted the pi-e-liberation branch October 11, 2023 23:44
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.

2 participants