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

RFC: add BitsFloat abstract type, leverage dispatch for rounding modes #5007

Merged
merged 1 commit into from
Dec 19, 2013

Conversation

simonbyrne
Copy link
Contributor

This adds an intermediate abstract type below FloatingPoint for floating point bitstypes (which I've termed BitsFloat, however it might be better to change this to make it more visually distinct from BigFloat). It also changes the behaviour of the rounding mode functions to leverage dispatch, for example:

# for ordinary floating point types
set_rounding(BitsFloat,RoundUp)
# for BigFloats
set_rounding(BigFloat,RoundUp)

It still requires some documentation, there are options for further behaviour:

  • set_rounding(RoundUp) to change both modes
  • set_rounding(Float64,RoundUp) == set_rounding(BitsFloat,RoundUp)

else
error("Unknown Operating System")
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you meant for this to sneak in here.

@ViralBShah
Copy link
Member

I generally like this API cleanup.

@StefanKarpinski
Copy link
Member

If we're going to do this we may want to also have BitsInteger.

@StefanKarpinski
Copy link
Member

Which is relevant to @vtjnash's pr that had generic definitions of many integer operations.

@vtjnash
Copy link
Member

vtjnash commented Dec 4, 2013

Or allow specifying the type of the type in the function definitions

@JeffBezanson
Copy link
Member

I'm not sure about this type. There is not really any connection between being a bitstype and setting rounding modes. It probably makes the most sense for the fenv rounding modes to apply to Union(Float32,Float64).

@simonbyrne
Copy link
Contributor Author

@JeffBezanson The rationale was that the function changes the rounding mode of both types (or at least that's my understanding). But dispatching on concrete types is probably more useful (in fact that was my motivation for this).

If we do decide to keep it, perhaps IEEE754 might be a better name, since there might be other types for which we can't set rounding (such as a Float80 type?)

@kmsquire
Copy link
Member

kmsquire commented Dec 4, 2013

You could also make BitsFloat or IEEE754 an alias for
Union(Float32,Float64).

Kevin

On Wed, Dec 4, 2013 at 2:29 PM, Simon Byrne [email protected]:

@JeffBezanson https://github.com/JeffBezanson The rationale was that
the function changes the rounding mode of both types (or at least that's my
understanding). But dispatching on concrete types is probably more useful
(in fact that was my motivation for this).

If we do decide to keep it, perhaps IEEE754 might be a better name, since
there might be other types for which we can't set rounding (such as a
Float80 type?)


Reply to this email directly or view it on GitHubhttps://github.com//pull/5007#issuecomment-29852571
.

@simonbyrne
Copy link
Contributor Author

I've removed the abstract type, and replaced it by a union. Before I update the documentation, we need to decide on the behaviour of set_rounding(r::RoundingMode). The possible options that I can think of are:
a. leave undefined
b. set rounding of Float32/Float64 only
c. set rounding of Float32/Float64 and BigFloat

@simonbyrne
Copy link
Contributor Author

Okay, I've added docs and deprecation warnings (I've taken option b above, which corresponds to old usage).

@ivarne
Copy link
Member

ivarne commented Dec 11, 2013

NEWS.md

@simonbyrne
Copy link
Contributor Author

Okay, I've added the news and rebased.

JeffBezanson added a commit that referenced this pull request Dec 19, 2013
RFC: add BitsFloat abstract type, leverage dispatch for rounding modes
@JeffBezanson JeffBezanson merged commit eca63da into JuliaLang:master Dec 19, 2013
@simonbyrne simonbyrne deleted the bitsfloat branch March 10, 2015 12:00
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.

7 participants