Skip to content

Add assembly into rectangular sparse matrices#1279

Merged
termi-official merged 9 commits intomasterfrom
do/rectangular-assemble
Mar 7, 2026
Merged

Add assembly into rectangular sparse matrices#1279
termi-official merged 9 commits intomasterfrom
do/rectangular-assemble

Conversation

@termi-official
Copy link
Copy Markdown
Member

@termi-official termi-official commented Feb 6, 2026

Closes #1133 (and #968 ?)

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.13%. Comparing base (511e715) to head (3eb5097).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1279      +/-   ##
==========================================
- Coverage   94.23%   94.13%   -0.11%     
==========================================
  Files          40       40              
  Lines        6751     6766      +15     
==========================================
+ Hits         6362     6369       +7     
- Misses        389      397       +8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@termi-official termi-official force-pushed the do/rectangular-assemble branch from 3b2170f to c44f6bb Compare February 6, 2026 17:11
@termi-official termi-official force-pushed the do/rectangular-assemble branch from c44f6bb to e8e8be6 Compare February 6, 2026 17:12
@termi-official termi-official added the awaiting review PR is finished from the authors POV, waiting for feedback label Feb 6, 2026
Copy link
Copy Markdown
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 some minor comments regarding docs and tests, but looks good to me.

Could you post a benchmark in the PR comparing master and PR for assembling a constant Ke and fe with some grid, just so we document that there are no performance regressions (I assume there isn't?)

Comment thread src/assembler.jl
Comment thread test/test_assemble.jl Outdated
Comment thread test/test_assemble.jl Outdated
Comment thread test/test_assembler_extensions.jl Outdated
Comment thread test/test_assemble.jl
@KnutAM
Copy link
Copy Markdown
Member

KnutAM commented Feb 26, 2026

Another question, what about BlockAssembler (I don't know this code so well, but looking at it I guess this should be extended to that case as well?)

And side-note, but supertype(BlockAssembler) = AbstractAssembler, can this be both CSR and CSC? Or should that subtype AbstractCSCAssembler?

@termi-official
Copy link
Copy Markdown
Member Author

Another question, what about BlockAssembler (I don't know this code so well, but looking at it I guess this should be extended to that case as well?)

Potentially, yes, but right now I do not have a real use-case for it and also cannot think about one. So it could become a good first issue.

And side-note, but supertype(BlockAssembler) = AbstractAssembler, can this be both CSR and CSC? Or should that subtype AbstractCSCAssembler?

This looks correct to me. The block format is neither CSR nor CSC.

@KnutAM
Copy link
Copy Markdown
Member

KnutAM commented Feb 26, 2026

This looks correct to me. The block format is neither CSR nor CSC.

But the underlying matrix is one of them? For me it was just not clear if both are supported (perhaps I should just try but it is late 😄)

@termi-official
Copy link
Copy Markdown
Member Author

using Ferrite, SparseArrays, BenchmarkTools

Ke = zeros(4,4)
K = sprand(4,4,1.0)

assembler = start_assemble(K)
dofs = collect(1:4)
@btime assemble!($assembler, $dofs, $Ke)

This benchmark show no statistically significant difference between the PR and master.

@termi-official termi-official requested a review from KnutAM March 2, 2026 12:12
Copy link
Copy Markdown
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.

Sorry, missed the aliasing in the previous round and had some questions about that. I didn't notice that and that was the reason for the performance question since I figured with double the vectors there could be some additional costs.

Otherwise just some minor tweaks.

Comment thread src/assembler.jl Outdated
Comment thread src/assembler.jl Outdated
Comment thread src/assembler.jl Outdated
Co-authored-by: Knut Andreas Meyer <knutam@gmail.com>
Comment thread src/assembler.jl Outdated
@termi-official termi-official requested a review from KnutAM March 6, 2026 16:16
Copy link
Copy Markdown
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.

LGTM!

@termi-official termi-official merged commit a347761 into master Mar 7, 2026
15 of 16 checks passed
@termi-official termi-official deleted the do/rectangular-assemble branch March 7, 2026 09:53
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.

assemble! for non-square J missing

2 participants