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.

Sign up for GitHub

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

Jump to bottom

Integrate common floating point constants #194

Closed
aomader opened this issue Jun 8, 2016 · 7 comments
Closed

Integrate common floating point constants #194

aomader opened this issue Jun 8, 2016 · 7 comments

Comments

@aomader
Copy link
Contributor

aomader commented Jun 8, 2016

In most of my implementations involving generic floating point values I have the need to work with pi and/or e. Thus it would be really helpful to provide static functions to retrieve those values for the concrete type.
@cuviper
Copy link
Member

cuviper commented Jun 8, 2016

In general, let e = Float::one().exp(); and let pi = (-Float::one()).acos(); should work. We could provide these as default implementations and override the specific constants for f32 and f64.

@cuviper
Copy link
Member

cuviper commented Jun 8, 2016

Associated constants would be nice too, rust-lang/rust#29646, but we couldn't provide defaults there.

@IvanUkhov
Copy link
Contributor

I agree that pretty much every mathematical constant can be computed using the methods that are already there. However, (-Float::one()).acos() and alike obscure the actual algorithm. Do we really want to see people in the Rust community writing this kind of generic code? Besides, who is willing to guarantee that (-Float::one()).acos() is indeed a zero-cost abstraction?

@cuviper
Copy link
Member

cuviper commented Aug 11, 2016

No, I don't expect people should do that -- it's just a way to write a generic default so it's not a breaking change. We can provide the proper constants in the implementation for the concrete float types.

Care to write up a PR?

@IvanUkhov
Copy link
Contributor

@cuviper, first of all, I apologize if it sounded rude; no offense was intended. I’m an active user of num and really appreciate your work.

Yes, absolutely. If there are no objections, I’ll be happy to help.

@cuviper
Copy link
Member

cuviper commented Aug 11, 2016

No problem, sorry if I sounded too defensive. :)

@IvanUkhov IvanUkhov mentioned this issue Aug 11, 2016
Merged
homu added a commit that referenced this issue Aug 16, 2016
@homu
Auto merge of #220 - IvanUkhov:constant, r=cuviper
ccdf307 
Add a trait for floating-point constants

The pull request is to address issue #194. In order to keep the library organized, I’ve introduced a new trait for the new functionality. The trait is supposed to closely follows the [`consts`](https://doc.rust-lang.org/std/f64/consts/index.html) module from the standard library. There are at least the following three open questions:

1. What should the name of the trait be? Currently, it’s `Constant`; an alternative is `Consts`.
2. What should the names of the getters be? Currently, they are lower-case versions of the constants defined in the `consts` module. Note that `Float` provides `log2` and `log10`, whereas `LOG_2` and `LOG_10` get translated into `log_2` and `log_10`, respectively.
3. Should `Float` require the new trait? Or should it be left up to the user?

Please let me know what you think. Thank you!

Regards,
Ivan
@cuviper
Copy link
Member

cuviper commented Aug 16, 2016

Fixed by #220 -- thanks @IvanUkhov!

@cuviper cuviper closed this as completed Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@aomader @cuviper @IvanUkhov