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

Generic frexp #150

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Generic frexp #150

wants to merge 1 commit into from

Conversation

mcabbott
Copy link

Implements roughly this idea: #148 (comment)

@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #150 (950cea6) into master (d4a2e48) will increase coverage by 0.11%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
+ Coverage   48.75%   48.87%   +0.11%     
==========================================
  Files          63       63              
  Lines        3425     3429       +4     
==========================================
+ Hits         1670     1676       +6     
+ Misses       1755     1753       -2     
Impacted Files Coverage Δ
src/math/prearith/prearith.jl 58.40% <85.71%> (+3.36%) ⬆️

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 d4a2e48...950cea6. Read the comment docs.

@JeffreySarnoff
Copy link
Member

seems reasonable -- it is breaking though. thoughts?

@mcabbott
Copy link
Author

mcabbott commented May 13, 2022

I guess it's your call whether to call it a bugfix :)

No rush from me, certainly not worth 2.0.

You would have to depend on this package to be relying on the old behaviour. None of the direct packages in General seem to. What people do in private I don't know.


From Juliahub, list of deps:
https://juliahub.com/ui/Packages/DoubleFloats/KzTCm/1.2.2?page=2
Searching the direct ones, none contain frexp

Packages which do use or define frexp include:

https://github.com/JuliaArbTypes/ArbFloats.jl/blob/9710fb9b65adff5a62336b6eff50b4d55f378f75/src/basics/IEEEfp.jl#L22-L27

https://github.com/StevenWhitaker/BlochSim.jl/blob/f5a353867c2b08c2e40b6737e97586dad8882581/src/expm.jl#L200-L212
Defines it for Dual numbers

https://github.com/JuliaPhysics/Measurements.jl/blob/b26fbff340f6a9c1432a06c8941ff9ba9a0ca34f/src/math.jl#L580-L583
Defines it to pass measurements along

https://github.com/JuliaMath/Quadmath.jl/blob/154de7bd735946b4e8dba03426c7e3e81bc3fbd1/src/Quadmath.jl#L388-L392

https://github.com/PainterQubits/Unitful.jl/blob/960e09ef518b0d3ff54747460b9c3872c08df0d7/src/quantities.jl#L467-L470
Defines it to pass units along

Applies `Base.frexp` to both `hi` and `lo` parts, and returns a tuple of tuples.
See also `DoubleFloats.ldexps` for the inverse, and `DoubleFloats.signs` similar tuple.
"""
function frexps(x::DoubleFloat{T}) where {T<:IEEEFloat}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of an awful name, trying to match signs. Could be more explicit like double_frexp or something.

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.

2 participants