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

Faer dense matrix #21

Merged
merged 13 commits into from
Apr 5, 2024
Merged

Conversation

Siel
Copy link
Contributor

@Siel Siel commented Apr 2, 2024

Implementation of the Matrix trait using the dense matrix representation in faer.

Tests are passing.

In order to get a fully working example we might need to generalize the LU struct

@Siel
Copy link
Contributor Author

Siel commented Apr 3, 2024

Tomorrow will rebase the branch and try to reuse as much code as I can using macros

@Siel
Copy link
Contributor Author

Siel commented Apr 3, 2024

This should close #7

@Siel Siel force-pushed the faer_dense_matrix branch from 5603d76 to 37b8666 Compare April 3, 2024 20:51
@Siel Siel force-pushed the faer_dense_matrix branch from b244ca0 to 2c80934 Compare April 5, 2024 10:24
@Siel
Copy link
Contributor Author

Siel commented Apr 5, 2024

@martinjrobins Please check this PR, here we are adding support for Faer's dense matrix representation. All tests are passing, the code is rebased and I collapsed the arithmetic implementations of for Matrix and Vector as much as I could using Macros.

I also fixed an error in one of the macros for Sundials

@martinjrobins
Copy link
Owner

great, thanks @Siel. I've just had a quick glance and will have a proper look later. quick question: I notice that all the faer types use f64, is it possible to make this a generic parameter T? this would be much more useful

@Siel
Copy link
Contributor Author

Siel commented Apr 5, 2024

Sure, I can do it

@Siel
Copy link
Contributor Author

Siel commented Apr 5, 2024

@martinjrobins Done

Copy link
Owner

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff, thanks @Siel. Minor changes, plus I think that From<faer::Scale> should be implemented for Scale<T> to neaten up the code

@Siel Siel requested a review from martinjrobins April 5, 2024 14:27
Copy link
Owner

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, happy to merge

@martinjrobins martinjrobins merged commit ce37930 into martinjrobins:main Apr 5, 2024
6 checks passed
@martinjrobins martinjrobins deleted the faer_dense_matrix branch April 5, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants