Skip to content

Fix fallback sign() issues in Base implementation - #360

Merged
kimikage merged 1 commit into
JuliaMath:masterfrom
kimikage:fix_sign
Oct 17, 2026
Merged

kimikage merged 1 commit into
JuliaMath:masterfrom
kimikage:fix_sign

Conversation

@kimikage

Copy link
Copy Markdown
Collaborator

Since the fallback implementation attempts to generate out-of-range values to avoid branching, it throws an error regardless of whether the value is in-range or not.

julia> sign(0.0N0f8)
ERROR: ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent -1 # before
0.0N0f8 # after

julia> sign(0.0Q0f7)
ERROR: ArgumentError: Q0f7 is an 8-bit type representing 256 values from -1.0 to 0.992; cannot represent 1 # before
0.0Q0f7 # after

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.11%. Comparing base (f979b14) to head (66ba27b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
- Coverage   97.22%   97.11%   -0.12%     
==========================================
  Files           7        7              
  Lines         792      796       +4     
==========================================
+ Hits          770      773       +3     
- Misses         22       23       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kimikage

Copy link
Copy Markdown
Collaborator Author

Considering this issue hasn't been reported for a long time, sign() may be unused actually, though.

@kimikage

kimikage commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

Surprisingly, (::Type{X})(x::X) where {X <: FixedPoint} was being called by the fallback method. (I don't think this is an oversight in the test. It's probably the result of constant folding.)
The shortcut might not actually be necessary. (Untested)

Edit:
As for Fixed and Normed, it appeared that the absence of the method did not affect execution speed.
However, it should probably not be removed, as it serve as a fallback in case users or add-on packages define their own FixedPoint types.

@kimikage
kimikage merged commit e08cf98 into JuliaMath:master Oct 17, 2026
14 of 15 checks passed
@kimikage
kimikage deleted the fix_sign branch October 17, 2026 04:56
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