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

SparseMatrixCSR extension #864

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

termi-official
Copy link
Member

@termi-official termi-official commented Dec 23, 2023

Towards #848 and #628 . This should show how we could assemble into custom sparse matrix formats.

TODO

  • Fix inhomogeneous boundary conditions
  • Add devdocs
  • Add test

@termi-official termi-official force-pushed the do/sparsematrixcsr-assembly-extension branch from 49d273a to 27d1980 Compare December 28, 2023 01:05
@termi-official termi-official force-pushed the do/sparsematrixcsr-assembly-extension branch from 27d1980 to 0c3efc8 Compare December 28, 2023 01:17
@codecov-commenter
Copy link

codecov-commenter commented Dec 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.70%. Comparing base (b56e2c9) to head (0f42594).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
+ Coverage   93.57%   93.70%   +0.12%     
==========================================
  Files          39       40       +1     
  Lines        6074     6160      +86     
==========================================
+ Hits         5684     5772      +88     
+ Misses        390      388       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@termi-official termi-official marked this pull request as ready for review December 28, 2023 01:41
@termi-official termi-official added the awaiting review PR is finished from the authors POV, waiting for feedback label Dec 28, 2023
@termi-official termi-official removed the awaiting review PR is finished from the authors POV, waiting for feedback label Dec 28, 2023
@termi-official termi-official marked this pull request as draft December 28, 2023 02:26
@termi-official termi-official force-pushed the do/sparsematrixcsr-assembly-extension branch from 94ece96 to 8102774 Compare December 28, 2023 03:20
@termi-official termi-official marked this pull request as ready for review December 28, 2023 03:41
@termi-official termi-official added the awaiting review PR is finished from the authors POV, waiting for feedback label Dec 28, 2023
@termi-official termi-official changed the title SparseMatrixCSR extension baseline SparseMatrixCSR extension Dec 28, 2023
Copy link
Member

@KnutAM KnutAM left a comment

Choose a reason for hiding this comment

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

Just took a look as I was curious, nice work on factoring out some parts - this will make it easier to extend!

ext/FerriteSparseMatrixCSR.jl Outdated Show resolved Hide resolved
ext/FerriteSparseMatrixCSR.jl Outdated Show resolved Hide resolved
src/Dofs/ConstraintHandler.jl Outdated Show resolved Hide resolved
@termi-official termi-official mentioned this pull request Jan 29, 2024
Copy link
Collaborator

@lijas lijas left a comment

Choose a reason for hiding this comment

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

Nice!
It could be nice to use the SparseMatrixCSR in one of the tutorials also (for testing and showing user that it is possible)

ext/FerriteSparseMatrixCSR.jl Outdated Show resolved Hide resolved
@fredrikekre
Copy link
Member

I suppose in all(?) cases the solver will dictate what matrix you need to, or can, use. Is there any solver that can handle SparseMatrixCSR but not SparseMatrixCSC?

@termi-official
Copy link
Member Author

Nice! It could be nice to use the SparseMatrixCSR in one of the tutorials also (for testing and showing user that it is possible)

I decided to not use it in one of the tutorials for now, because I didn't wanted to clutter any of these. It should also be clear how to use it from the docstring of start_assemble ? Which test is missing in https://github.com/Ferrite-FEM/Ferrite.jl/pull/864/files#diff-7fa26eddc1fe6a8a5341e0fcdffb1d8c256e6a566819cfcf70b625ef80ab6abd ? (At least codecov says I have coverage).

@termi-official
Copy link
Member Author

I suppose in all(?) cases the solver will dictate what matrix you need to, or can, use. Is there any solver that can handle SparseMatrixCSR but not SparseMatrixCSC?

If you talk about matrix formats, then e.g. Ginkgo and Hypre just have CSR formats (and no CSC).

@fredrikekre
Copy link
Member

Yea but then you wouldn't go through SparseMatrixCSR.jl but directly to a HYPRE matrix or Ginkgo matrix?

@termi-official
Copy link
Member Author

Yea but then you wouldn't go through SparseMatrixCSR.jl but directly to a HYPRE matrix or Ginkgo matrix?

Yes

Comment on lines +21 to +25
```
Ferrite.start_assemble!
Ferrite.finish_assemble!
Ferrite.assemble!
```
Copy link
Member Author

Choose a reason for hiding this comment

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

What is the correct way to communicate this? Adding @docs breaks the CI, because the doc string is already listed in references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review PR is finished from the authors POV, waiting for feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants