Skip to content

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Aug 13, 2025

The idea for this addition to the test suite originates here, where stevengj suggested to do this test for cosc specifically:

This PR instead tests all applicable functions I could find.

Also fix the sign of cosc(::BigFloat) for zero input, thanks to stevengj.

The idea for this addition to the test suite originates here, where
stevengj suggested to do this test for `cosc` specifically:

* JuliaLang#59087 (comment)

This PR instead tests all applicable functions I could find.
@nsajko nsajko added test This change adds or pertains to unit tests maths Mathematical functions labels Aug 13, 2025
@nsajko
Copy link
Contributor Author

nsajko commented Aug 13, 2025

cosc(::BigFloat) is broken currently at the origin when testing it like this. Thus this PR doesn't test BigFloat, that can be left for a future PR.

@nsajko nsajko marked this pull request as ready for review August 13, 2025 21:44
@nsajko nsajko requested a review from stevengj August 13, 2025 21:44
@stevengj
Copy link
Member

cosc(::BigFloat) is broken currently at the origin when testing it like this.

Can be fixed by taking the line s = (term = -(π*x))/3 in the cosc(::Number) implementation and adding

iszero(s) && return s

immediately afterwards.

(Otherwise it computes one extra term in the Taylor series, which messes up the sign of zero because -0.0 + 0.0 === 0.0.)

@stevengj
Copy link
Member

stevengj commented Aug 13, 2025

Should it also test binary operators, like Base.Fix1(*, one(typ)), Base.Fix2(*, one(typ)), Base.Fix2(/, one(typ))?

@stevengj
Copy link
Member

This looks questionable:

julia> div(-0.0, 1.0)
0.0

@nsajko nsajko requested a review from oscardssmith as a code owner August 14, 2025 09:12
@stevengj
Copy link
Member

stevengj commented Aug 15, 2025

Should be good to merge.

In principle, we could backport the cosc(big"0.0") sign fix? Maybe too minor to bother with.

@nsajko nsajko added the bugfix This change fixes an existing bug label Aug 15, 2025
@JeffBezanson JeffBezanson merged commit fd52126 into JuliaLang:master Aug 15, 2025
9 checks passed
@nsajko nsajko deleted the test_monotonic_fp_functions_mapping_zero_to_zero branch August 16, 2025 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug maths Mathematical functions test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants