dimforge / alga

Abstract algebra for Rust.
194 stars 39 forks source link

decimal::d128 with alga::general::real #49

Open snowkeep opened 6 years ago

snowkeep commented 6 years ago

I suspect this is more of a usage error than a bug, but can't anywhere else to go for help.

I'm trying to use the ncollision2d and decimal crates, and keep getting this error:

error[E0277]: the trait bound `decimal::d128: alga::general::Real` is not satisfied
  --> src/misc.rs:86:3
   |
86 |   pub cuboid: Cuboid<d128>,
   |   ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `alga::general::Real` is not implemented for `decimal::d128`
   |
   = note: required by `ncollide2d::shape::Cuboid`

Among others, I have this in my Cargo.toml:

[dependencies]
ncollide2d = "0.17.1"
decimal = "2.0.4"

[dependencies.alga]
version = "0.7.1"
features = ["decimal"]

As far as I can tell, that should enable the Real trait for decimal, but it isn't working. Is this a problem with alga? If not, can you tell me what I'm missing?

Thanks.

WaDelma commented 6 years ago

If you check Cargo.lock is there only one version of alga there? It could be that ncollide has different version which would cause this.

snowkeep commented 6 years ago

Just 0.7.1. I also tried deleting the Cargo.lock, and running cargo clean, cargo build and confirmed that only the one version gets compiled.

sebcrozet commented 6 years ago

@snowkeep There are two issues here:

  1. It appears the #[cfg] guards related to d128 on alga are plain wrong. So enabling the decimal feature actually has no effect. I will fix this now.
  2. Unfortunately the Real trait cannot be implemented for d128 because of missing trait implementations on the decimal crate. See https://github.com/alkis/decimal/issues/21 So, even after fixing the first issue mentioned here, the Real implementation will not be available as long as the num traits are not implemented for d128. For the record, the missing traits seem to be Bounded, Signed, FromPrimitive so they should not be too difficult to implement.
snowkeep commented 6 years ago

@sebcrozet. Thank you. I can't attempt to add the traits to d128 immediately, so I'll work on the rounding errors with f64 types in my code, for now. But I do intend to create a test-case and work on making d128 compatible. It's too useful to not have it.

snowkeep commented 6 years ago

I've added what looked like the necessary traits (num_traits::{Bounded, Signed, FromPrimitive and a hanful of other} and approx::{UlpsEq, RelativeEq and AbsDiffEq} in a branch called alga_support. I punted on from_str_radix for now (returns Ok(d128!(0.0)) - it's the most complicated method, by far, and I don't see a need for it, so want to test everything else before taking the time. Tests need to be added, as well. I modified a clone of alga to point to this. Still a couple of issues. It looks like alga::general::{AbstractField, SubsetOf, Lattice} traits are missing. I'll keep looking, but am going to need some help. If anybody wants to look at it, my test case is here: https://github.com/snowkeep/d128_alga. Watch out for the relative paths to a local alga and decimal in the Cargo.toml.

sebcrozet commented 6 years ago

Given the changes you've proposed on the decimal crate, I'm reopening this. I will see now what can be done using your changes.

sebcrozet commented 6 years ago

Thank you for your changes on decimal @snowkeep. Unfortunately, a few features (which are likely hard to implement) are still missing. You will find Real implemented for d128 on that branch: https://github.com/rustsim/alga/tree/d128 (and you need to enable alga's decimal feature).

Unfortunately, lots of methods of the Real trait are currently unimplemented!() because I could not find the corresponding methods on the decimal crate. Most of them are not actually necessary for ncollide itself, but some are essential: sin, cos, tan, asin, acos, atan2, sqrt, floor, ceil, round.

Also all the mathematical constants appear to be missing but I guess we could just convert constants from f64 to d128 for a first easy implementation (for example d128!(f64::consts::PI) for the ::pi() constant).

Finally, I could not find a way to convert from d128 to lower-precision floats like f64 so the SubsetOf<d128> contains an unimplemented!() too (but this should not be used by ncollide either).