search

rustgd / cgmath

A linear algebra and mathematics library for computer graphics.
https://docs.rs/cgmath
Apache License 2.0
1.12k stars 155 forks source link

Fix compilation of benchmarks #505

Closed nstoddard closed 4 years ago

nstoddard commented 4 years ago

This adds the rand_isaac dependency since that's no longer in the rand crate, but in the future we should consider switching to an RNG in rand so we don't need another dependency.

kvark commented 4 years ago

Note: there is another error on nightly that's still happening: https://travis-ci.org/github/rustgd/cgmath/jobs/694474980#L222

Could this PR address this one too? Looks related

nstoddard commented 4 years ago

It looks like that error is due to the use of an old version of rust nightly (which is needed for SIMD support) which rand_core doesn't support. Optional dev-dependencies aren't supported, so one way to fix this would be to move rand_isaac back to a regular dependency so it can be enabled for benchmarks but not tests, but that's not ideal since it makes it possible for other crates to enable that dependency.

Another way would be to switch to an RNG that doesn't require another dependency. Are any in rand suitable for benchmarks?

kvark commented 4 years ago

Did you consider using https://docs.rs/rand/0.7.3/rand/rngs/struct.SmallRng.html for this?

nstoddard commented 4 years ago

SmallRng requires a feature to be enabled, which would then also be enabled for any crates that enable cgmath's rand feature; is that okay? If not, would StdRng work?

kvark commented 4 years ago

I think we can enable it for rand, yes. Presumably, other crates enabling rand would be interested in using this.

nstoddard commented 4 years ago

Done.

kvark commented 4 years ago

Thank you! let's just see what CI thinks :)

kvark commented 4 years ago

SIMD is still broken - https://travis-ci.org/github/rustgd/cgmath/jobs/694761384#L220

nstoddard commented 4 years ago

That's a different one than the one that was broken by this PR (current nightly rather than nightly-2019-01-01), and it won't work until #490 is implemented.

kvark commented 4 years ago

Yeah, I figured, hence this PR is merged :)