Closed JamieBallingall closed 2 years ago
Generally, one concern core libraries have is whether adding some functionality will make it harder for non-JS backends to support that.
I think this library is in a weird place, but I don't have good answers for these questions either. I used to be a supporter of "it's just the Math object in JS", but that's muddied with ES6/2021 and the non-JS additions you mentioned. It seems like if we were going to move away from that, it should perhaps implement at least some things over our numeric classes rather than being specialised to Number
too.
I almost never find myself using this library, due to the nature of the things I work on these days, so my opinion probably shouldn't hold as much weight as those who are heavy users of it.
I think that in the past, having core libraries closely follow JS APIs made sense, but now that PureScript is more mature and especially now that there are alternative backends, we shouldn’t follow JS APIs too closely. For example, the strings and globals libraries have been moving away from closely following the JS API more recently. In my view the most important concern is whether we have a well designed library which allows PureScript users to do the things they need to do with a minimum of fuss.
I personally think that we could consider giving this library a similar treatment as globals. Most of the functions here could sensibly live in Data.Number, particularly the trig functions and constants and the logarithm stuff. One or two are obsoleted by more general functions which exist in Prelude - namely min
, max
, and abs
. The imul
function is unique in that it works on Int rather than Number, so that could go into Data.Int (or ideally into Int’s Semiring instance, but that’s slightly harder, see https://github.com/purescript/purescript/issues/2980).
I am hesitant to introduce a type class for trig functions or logarithms, because it’s hard for me to imagine demand for them. I’m not aware of numeric types other than Number which would support those operations. Haskell needs the Floating class because it has both Float and Double, but we only have Number. It is slightly easier to imagine type classes for sqrt and rounding functions, but I think that can be addressed separately from the question of where these functions should live.
I agree with @hdgarrood regarding type classes. Many of the functions currently in Math would require new type classes. You can't write sqrt :: Field a => a -> a
because rationals are a field but do not support sqrt
. It seems silly to have type classes whose only instances are Number
and, even then, that instance isn't fully law abiding. Even min
and max
are different to the Data.Ord versions -- specifically in their handling of NaN. I think we preserve all these number-only versions in Data.Number until someone implements algebraic numbers or something where a type class would occur naturally.
From this discussion is sounds like: (A) We should prioritize supporting multiple backends by minimizing the number of functions a new backend needs to implement (B) We should move various functions into more sensible places, likely resulting in this library being deprecated at some point
I'd like to add a third priority, if I may: (C) We should avoid doing anything that makes writing high-performance numeric code harder than it is now
These suggest the following steps:
foreign import pi :: Number
we have pi = 3.141592653589793
. We would have to check that our constants are identical to the Javascript native ones but that should be easy.acos
, asin
, atan
, sin
, cos
, tan
, exp
, log
, pow
,sqrt
, and remainder
/%
. All of these are likely to be available in standard math libraries for most backends and would require fairly complex Purescript implementations.min
, max
, abs
and sign
. That's only four functions and backends are likely to have easily available versions.imul
, round
, ceil
and floor
. The versions of round
, ceil
and floor
already in Data.Int are currently implemented in terms of Math. Only the Data.Int versions, with type Number -> Int
would be available after Math is deprecated. Users wanting Number -> Number
versions would have to convert back from Int
, which I think is fine. trunc
would be implemented in Purescript in terms of ceil
and floor
in Data.Intatan2
is implemented, Math could be depreciated. Functions from ES6 and others could be implemented at our leisure.Together this would reduce the burden on new backends by reducing the number of functions they need to implement slightly versus the current situation (dropping all constants, atan2 and trunc) and dramatically versus the ES6-with-polyfills approach. By eventually depreciating Math it also removes a psychological association with Javascript. The functionality currently demanded in various issues and pull-requests would eventually be provided, albeit without a definite timetable.
I was going to suggest something about checking that atan(y/x)
behaves identically to atan2(y, x)
... but actually I think that's overkill - I'd be pretty surprised if there is an observable difference between the two that anyone is depending on, and if they are I'd suggest a separate JS-specific library / project-local FFI function can be introduced to deal with that again.
So yeah, the proposal above sounds good to me!
I agree, except:
Number -> Number
rounding functions; I think they are useful often enough that they are worth providing, and also the range of Number is much larger than that of Int, so it’s not possible to recover the same behaviour for numbers outside of the Int range.atan2
should be one of the “core” math functions, because you’d need to branch to implement it in terms of atan
. atan2(y, x)
is not the same as atan(y/x)
because the latter can’t return angles in all four quadrants; see https://stackoverflow.com/a/12011762Can you please list the other “non-core” functions which we’d implement in PureScript, just to make sure we are on the same page?
Oh also, I think constants don’t need to go in a separate module. I think it would be annoying to have to add another import just to use pi, so I’d personally rather they just go in Data.Number.
Ah, great point on atan2
, I had a dormant inclination there was something special about it, hence my earlier comment, but couldn't dredge up why it actually mattered.
Ok. I agree we can:
ceil
, floor
and round
in Data.Number and have the Data.Int versions use those"Non-core" functions would come in three phases:
atan2
in Data.Number (if we go that way) and trunc
in Data.Int.acosh
, asinh
, atanh
, cosh
, sinh
, atanh
), logs to other bases (log2
and log10
) and numerical stability functions (hypot
, log1p
and expm1
). I don't see a need for cbrt
, clz32
or fround
but they could be implemented if someone is interested (and being able to leave them out is an advantage of moving away from the "it's just the Math object in JS" approach). The repo purescript-math-es6
has Javascript polyfills taken from MDN for all these functions which could be fairly easily converted to Purescript but I'd like to take the process slow and ensure that only very robust and stable versions are implemented.Number -> Number
and are general enough that they probably belong in Data.Number. I would oppose putting the cdf or pdf of any particular probability distribution in Data.Number, however.Wikipedia suggests a branchless implementation of atan2
as
((sign x) ^ 2) * atan(y / x) + (1 - sign y + (sign y) ^ 2) * pi / 2.0
.
I haven't checked this in detail yet.
Can we leave atan2
in the category of "branchless Purescript if a robust implementation is available, provided by the backend otherwise". I'll try to come up with a PR at some point just for a branchless atan2
and you guys can accept or reject it at that point based on its merits.
Or is that not worth it and we just punt it off to the backend?
Screw it. The Wikipedia approach gives atan2(0, 0) == NaN
whereas both the Node and Chrome versions give atan2(0,0) == 0
. Building an exactly matching branchless version will be a pain so I recant and now agree with @hdgarrood that atan2
should be implemented by the backend.
Summarizing https://github.com/purescript/purescript-math/issues/31#issuecomment-899776481, it seems that to fully deprecate this repo, we need to
Add/change the following to numbers
:
ceil
, floor
, and round
- done in https://github.com/purescript/purescript-numbers/pull/18atan2
(needed to deprecate purescript-math
) - done in https://github.com/purescript/purescript-numbers/pull/18acosh
, asinh
, atanh
, cosh
, sinh
, atanh
log2
and log10
hypot
, log1p
and expm1
Add/change the following to integers
:
ceil
, floor
, and round
, which use the numbers
version before convert back to Int
trunc
(needed to deprecate purescript-math
) - this needs to be added in a new PRThe rest is something that could be done but isn't needed.
Functions that could belong in numbers
:
Up for grabs / could be implemented by other libraries:
cbrt
, clz32
or fround
All code in math
has been ported to purescript-numbers
and purescript-integers
I would be good to nail down some questions regarding the scope of this library. Specifically:
The function
Math.random()
is not supported and I think that leaving it out is uncontroversial.On 1, the library currently supports the ES6 functions
imul
andtrunc
with polyfills but no others. So we have some options: 1a. Strictly no ES6. Removeimul
andtrunc
1b. No changes. Leaveimul
andtrunc
but add no others 1c. All ES6. Add all missing functions (exceptrandom
). This would fix issue #25.There are currently two open PRs (#19 and #21) that contribute some ES6 functions. They have been open for some time. I'm not sure why they haven't been accepted but the polyfills have some overflow issues. I recently released purescript-math-es6 which contains all ES6 functions with robust polyfills. The project purescript-math-es6-quality lets you see how well the polyfills match native values in Node.js and the browser. purescript-math-es6 could be merged into this library if full ES6 support is required.
My vote is 1c achieved by merging purescript-math-es6. But I am biased.
On 2, the library currently supports the function
remainder
/%
and the valuetau
which are not properties or methods of the Math object. PR #29 would add the golden ratio as another constant. Similar to 1 we have three options: 2a. Strictly Math properties and methods. Removeremainder
andtau
. Removingremainder
would be a significant breaking change 2b. No changes. Leaveremainder
andtau
but reject the golden ratio and make it clear in the README that other math functions and constants will also be rejected 2c. Accept the golden ratio and update the README asking for additional math functionalityMy vote here is 2b. I think the library should just reflect the JS Math object but the changes from adopting
2a
would be too painful. Perhaps we could moveremainder
toData.Number
and add depreciation warnings toremainder
andtau
so we can eventually remove them.Thoughts?