Skip to content

Fix % Normed{UInt32} on ARM and Improve NaN % FixedPoint #223

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

Merged
merged 1 commit into from
Aug 29, 2020

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Aug 26, 2020

Fixes #134 (#134 (comment))

julia> 1.0 % N0f32
0.4999999999N0f32 # before (on ARM)
1.0N0f32 # after

This also adds tests for NaN/Inf. This clarifies the behavior regarding NaN and Inf, which had not been explicitly defined.

This reduces the environment-/optimization-dependent instability of the results for NaN.

  Expression: NaN % N === NaN32 % N === zero(N)
   Evaluated: 65537.0N16f16 === 65537.0N16f16 === 0.0N16f16 # I could not reproduce this on my local machine.

(cf. https://github.com/JuliaMath/FixedPointNumbers.jl/runs/1029971789?check_suite_focus=true#step:5:26)
However, the rem is still affected by the undefined behavior of unsafe_trunc.

This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
@kimikage kimikage changed the title Fix % Normed{UInt32} on ARM Fix % Normed{UInt32} on ARM and Improve NaN % FixedPoint Aug 26, 2020
@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #223 into master will increase coverage by 0.50%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #223      +/-   ##
==========================================
+ Coverage   91.29%   91.79%   +0.50%     
==========================================
  Files           6        6              
  Lines         563      573      +10     
==========================================
+ Hits          514      526      +12     
+ Misses         49       47       -2     
Impacted Files Coverage Δ
src/fixed.jl 95.48% <100.00%> (+0.21%) ⬆️
src/normed.jl 93.26% <100.00%> (+1.11%) ⬆️
src/utilities.jl 96.29% <100.00%> (+0.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ca0d9d...da5f533. Read the comment docs.

@kimikage
Copy link
Collaborator Author

This is not the main topic of this PR, but the speed performance of % Fixed{Int8} was improved.

julia> x_f32 = rand(Float32, 1000, 1000);

julia> @btime $x_f32 .% Q0f7;
  8.012 ms (2 allocations: 976.70 KiB)   # v0.8.4
  762.600 μs (2 allocations: 976.70 KiB) # PR #220 
  125.699 μs (2 allocations: 976.70 KiB) # this PR

Of course this makes it easier to overflow as a trade-off, but still has enough range for practical purposes.

@kimikage
Copy link
Collaborator Author

This change is somewhat ugly, but I will merge this first as it is needed to implement the "new" division.

@kimikage kimikage merged commit a142308 into JuliaMath:master Aug 29, 2020
@kimikage kimikage deleted the test_nan branch August 29, 2020 01:07
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 30, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 30, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
…th#223)

This reduces the environment-/optimization-dependent instability of the results for `NaN`.
This also adds tests for `NaN`/`Inf`.
This clarifies the behavior regarding `NaN` and `Inf`, which had not been explicitly defined.
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.

rem returns zero for negative float input on ARMv7
1 participant