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

Use Swift Numerics for elementary functions #28

Merged
merged 1 commit into from Oct 19, 2020

Conversation

stephentyrone
Copy link
Member

This drops the canImport darwin/glibc/etc dance, which is ugly, and lets us use root instead of exp(log(x)/k), which is more accurate in some cases (but mainly is just nicer to read).

The downside is that this introduces a dependency for Algorithms, where previously it had none. I think that Numerics is an OK thing to depend on (especially for these functions, which are available from 0.0.1), but it is a massive increase in the number of dependencies that the package has, and we should give that some thought.

Also lets us use root instead of exp(log(x)/k), which is more accurate in some cases (but mainly is just nicer to read).

The downside is that this introduces a dependency for Algorithms, where previously it had none. I think that Numerics is an OK thing to depend on (especially for these functions, which are available from 0.0.1), but it _is_ a _massive increase_ in the number of dependencies that the package has, and we should give that some thought.
@natecook1000
Copy link
Member

I'm definitely reticent to add this due to the, as you say, massive increase in dependencies. It seems like there's a trade-off at some point — if Algorithms were to use more of the higher-level functionality, or the generic capability that Swift Numerics provides, it would make sense to bring in the dependency, but at this point it isn't worth the addition. What do you think?

@stephentyrone
Copy link
Member Author

stephentyrone commented Oct 19, 2020

I think it makes sense because:
(a) We want people to use SN for the elementary functions--it's the "blessed" package for those APIs. If Numerics needed Cycle or whatever, I would expect to pull in Algorithms for it.
(b) In this particular case, I expect that Algorithms will want other pieces of Numerics in the future (especially stuff around RNGs), so I believe that you'll end up with this dependency sooner or later anyway.

Oh, and:
(c) If we ever want to get the same results across platforms for a given PRNG (which I expect we will), the only path to that is via Numerics. We could choose to provide portable log and root operations there, but the Darwin/Glibc/etc operations will never give the same results everywhere.

@natecook1000
Copy link
Member

(c) If we ever want to get the same results across platforms for a given PRNG (which I expect we will), the only path to that is via Numerics. We could choose to provide portable log and root operations there, but the Darwin/Glibc/etc operations will never give the same results everywhere.

Sold on this point!

@natecook1000 natecook1000 merged commit a0b7954 into apple:main Oct 19, 2020
@stephentyrone stephentyrone deleted the use-the-package-luke branch October 20, 2020 14:34
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.

None yet

2 participants