Adding support for faer #12
Conversation
There was a problem hiding this comment.
thanks for adding this @Siel, really great work! I've made a few suggestions/questions below.
|
I think I made al the changes requested. |
|
I did rebase the branch to include the current changes on main |
src/scalar/mod.rs
Outdated
| } | ||
|
|
||
| //TODO: Not sure why it is now working | ||
| // impl<'a, E, V> Mul<Scale<E>> for V |
There was a problem hiding this comment.
this comment can be removed now?
There was a problem hiding this comment.
I'll give it a shot at implementing the other side of Mul tomorrow. After that I'll remove that comment
There was a problem hiding this comment.
Nop, I wanted to allow Scale to be on the Lhs of * but I was not able to. I might revisit this later.
There was a problem hiding this comment.
ok, for now just create an issue for this as a reminder
src/vector/faer_serial.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl<'a, T: Scalar> Div<crate::scalar::Scale<T>> for faer::Col<f64> { |
There was a problem hiding this comment.
| impl<'a, T: Scalar> Div<crate::scalar::Scale<T>> for faer::Col<f64> { | |
| impl<'a, T: Scalar> Div<Scale<T>> for faer::Col<f64> { |
src/vector/faer_serial.rs
Outdated
|
|
||
| impl<'a, T: Scalar> Div<crate::scalar::Scale<T>> for faer::Col<f64> { | ||
| type Output = faer::Col<f64>; | ||
| fn div(self, rhs: crate::scalar::Scale<T>) -> Self::Output { |
There was a problem hiding this comment.
| fn div(self, rhs: crate::scalar::Scale<T>) -> Self::Output { | |
| fn div(self, rhs: Scale<T>) -> Self::Output { |
src/vector/faer_serial.rs
Outdated
| use super::{Vector, VectorCommon, VectorIndex, VectorView, VectorViewMut}; | ||
| impl<'a, T: Scalar> Mul<Scale<T>> for ColRef<'a, f64> { | ||
| type Output = Col<f64>; | ||
| fn mul(self, rhs: Scale<T>) -> Self::Output { |
There was a problem hiding this comment.
these three mul's can be combined into a single macro
There was a problem hiding this comment.
I think only the two first Mul's can be combined here. The third one is not expecting a lifetime. and for the Div one the function implementation is too different, I should be able to pass an anonymous function to the macro but that seems too convoluted, no?
There was a problem hiding this comment.
ok, I don't want to make it too convoluted, so feel free to leave as is
src/vector/faer_serial.rs
Outdated
| } | ||
| impl<'a, T: Scalar> MulAssign<Scale<T>> for ColMut<'a, f64> { | ||
| fn mul_assign(&mut self, rhs: Scale<T>) { | ||
| let scale = faer::scale(rhs.value().into()); |
There was a problem hiding this comment.
same with the mul_assigns. Do the two bodies need to be different?
There was a problem hiding this comment.
They have to, since faer implements the multiplication for some types and not for others
src/vector/nalgebra_serial.rs
Outdated
| self * rhs | ||
| } | ||
| } | ||
| impl<'a, T: Scalar> Mul<Scale<T>> for DVectorView<'a, T> { |
There was a problem hiding this comment.
same with faer_serial, use macros to reduce the repeated code
There was a problem hiding this comment.
The same here, I managed to create one macro, the other functions are different enough.
There was a problem hiding this comment.
ok, if a macro doesn't work here then don't worry about it.
There was a problem hiding this comment.
I might be able to create a more generic macro but it will take me some more time, I'll create a reminder to come back here later. I want to implement Matrix first
I'd like so, if other people is going to work on different parts of the library would be better to have the Scale struct on the main branch so rebasing does not become a nightmare |
This PR aims to add support for
faerintodiffsol.To do this I created a
Scalestructure to be used as an interface betweennalgebraandfaerwhen working with scalar operations.I replaced all the scalar operations to use
Scaleand implemented all the traits needed to have it to work withnalgebra.All tests are passing.
The
faertraits are still under development but the initial integration is already in place. All thefaerintegration code is commented to guarantee that the package compiles. My plan is to put the respective implementations behind their specificfaerandnalgebrafeatures.