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

Remove 'static bound from type A in CorrelationExt.cov #3

Open
LukeMathWalker opened this issue Sep 21, 2018 · 3 comments
Open

Remove 'static bound from type A in CorrelationExt.cov #3

LukeMathWalker opened this issue Sep 21, 2018 · 3 comments
Labels
On Hold Issues we can't act upon due to external constraints (e.g. missing Rust feature, other libraries..)

Comments

@LukeMathWalker
Copy link
Member

As soon as rust-ndarray/ndarray#491 is merged in ndarray.

@LukeMathWalker LukeMathWalker mentioned this issue Sep 21, 2018
17 tasks
@jturner314
Copy link
Member

rust-ndarray/ndarray#491 is now merged.

@jturner314
Copy link
Member

I was wrong that removing the A: LinalgScalar bound on .mean_axis() would allow us to remove the need for the A: 'static bound on .cov(). The call to .dot() (let covariance = denoised.dot(&denoised.t());) has the same issue.

We can't remove the 'static bound from .dot() without tradeoffs since the .dot() implementation relies on std::any::TypeId to specialize for f32 and f64. We have two options to remove the 'static bound on .dot() by changing ndarray:

  • Don't specialize for f32 and f64. This would prevent ndarray from using BLAS.
  • Perform the specialization through LinalgScalar trait implementations. This would work but would mean that LinalgScalar could no longer be implemented for T: (the various constraints); we'd have to implement LinalgScalar for individual types. This would mean that users of ndarray and another crate other could not use .dot() with types from other unless other or ndarray provided the LinalgScalar implementation.

Alternatively, we could to modify the Rust language/standard library to either (1) allow specialization of trait implementations (an RFC has been accepted for this but hasn't been implemented yet) or (2) remove the 'static bound on std::any::TypeId.

A final alternative is to provide another .dot() method in ndarray (named .dot_any() or something) that does not perform any specialization and so doesn't require the 'static bound. We'd need to then add .cov_any() as well. This approach isn't too bad but makes the API uglier.

In conclusion, I'd prefer to just keep the A: 'static bound for the time being until (1) someone has a specific use case where the 'static bound is violated (which I expect would be quite rare) or (2) the Rust language/standard library has the necessary improvements to remove the 'static bound without the disadvantages described above.

@LukeMathWalker
Copy link
Member Author

LukeMathWalker commented Sep 29, 2018

I agree with the conclusions of your analysis - waiting for rust-lang/rust#31844 to get implemented is probably the best course of action for now.
Unfortunately it is not going to be part of the 2018 Edition, hence we will probably have to wait a year or so to see any tangible progress on that RFC.

@LukeMathWalker LukeMathWalker added the On Hold Issues we can't act upon due to external constraints (e.g. missing Rust feature, other libraries..) label Sep 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Hold Issues we can't act upon due to external constraints (e.g. missing Rust feature, other libraries..)
Projects
None yet
Development

No branches or pull requests

2 participants