Razican / vsop87-rs

VSOP87 algorithm in Rust
https://crates.io/crates/vsop87
Apache License 2.0
16 stars 8 forks source link

Add f32 calculations #21

Open Razican opened 7 years ago

Razican commented 7 years ago

Currently all calculations are done in f64 types, but many users could benefit from less precise f32 calculations, mainly in platforms with smaller word sizes.

tonetheman commented 7 years ago

Any thoughts on names for f32 calculations?

Razican commented 7 years ago

I was thinking on using the same names as the f64 version but with _f32 at the end of the function names. Something like

vsop87::earth_moon_f32(jde:f32) -> (f32, f32, f32, f32, f32, f32)

for

pub fn earth_moon(jde: f64) -> (f64, f64, f64, f64, f64, f64)

What I'm not sure about is if we would need to add a new set of constants, or doing a lossy f64 -> f32 casting would be enough (is this done in compile time?)

Also, we would need to create new tests for this version.

We would also need to add the main functions src/lib.rs#L39 calculate_t_f32 and calculate_var_f32.

In any case, there is another option, which would probably be even better for the future #19. We could create a feature named f32 and another named f64 in the Cargo.toml. And the default would be f64. That would enable us to have two versions of each function, something like this:

#[cfg(feature = "f64")]
pub fn mercury(jde: f64) -> (f64, f64, f64, f64, f64, f64) {
. . .
}

#[cfg(feature = "f32")]
pub fn mercury(jde: f32) -> (f32, f32, f32, f32, f32, f32) {
. . .
}

This would also enable compile time optimizations in constants, by having two versions of the planet modules (mercury_f64.rs and mercury_f32.rs, for example) and when importing the modules, it would be as simple as doing something like this:

#[cfg(feature = "f32")]
mod mercury_f32;
#[cfg(feature = "f64")]
mod mercury_f64;

Both files would be an exact copy, just changing the header for each constant (pub const A0: [(f64, f64, f64); 496] to pub const A0: [(f32, f32, f32); 496].

I want to read your opinion though :)

tonetheman commented 7 years ago

I did this as a quick thing to to which you like better, this is the features

https://github.com/tonetheman/vsop87-rs/tree/f32_experiment

It is just a branch with the features added to Cargo.toml and one new (copied and changed) function in lib_32.rs

I did a build with this cargo build -v --features f32

I am new enough to rust to not exactly know how to even tell it is doing anything (ha). The tests still passed which made me think cargo test Was not really testing anything, once again this is my inexperience with rust showing...

Any thoughts?

Razican commented 7 years ago

You would need to include the feature in the cargo test command: cargo test --features f32. And, you would need to include both, the f32 and f64 versions in the same lib.rs file. Also, I would add "f64" as the default feature:

[features]
default=["f64"]
f32= []
f64 = []
tonetheman commented 7 years ago

alright i will look into that and see where i get!

tonetheman commented 7 years ago

Ok look at my branch now. Features (at least the default part) do not work like I had thought. The default feature appears to always be used. So when I have an f32 func and a f64 func defined with the same name when you do this command you will see both features on at once

cargo build --features f32 --verbose

I read you can set features as cherry picked but it looks like it is in the toml file?

Razican commented 7 years ago

Yes, if you don't want to compile default features, you need to specify --no-default-features. So, for compiling f32 it would be this command:

cargo build --no-default-features --features f32
tonetheman commented 7 years ago

alright thanks I got enough to go ahead then that was what I needed!

NiklasVousten commented 8 months ago

Is this feature still planned?

An approach like iliekturtles/uom implemented for multiple types could be used. They have a macro rule called storage_type!, which allows code duplication for multiple files, without the need of copy-pasting the code. storage_types.rs This would then result in a structure like this: vsop87::f32::earth_moon and vsop87::f64::earth_moon

NiklasVousten commented 8 months ago

To prevent a large amount of code duplications generics could be used. I made some tests in https://github.com/NiklasVousten/vsop87-rs/tree/f32

Currently only Mercury is capable of f32 operations, and the tests need to be modified to allow for smaller precision. Furthermore the simd functions werent adapted yet.

The constant arrays are currently converted. But with a duplication of those, this step could be skipped and thus have additional constant f32 arrays. Convertion would be less work though.

Any thoughts?