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

Cross-elements coupling for DiscontinuousLagrange sparsity patterns. #710

Merged
merged 71 commits into from
Jul 25, 2023

Conversation

AbdAlazezAhmed
Copy link
Collaborator

@AbdAlazezAhmed AbdAlazezAhmed commented May 15, 2023

It iterates over all cells twice, once to get the length of I,J and another to fill them. There is probably a better way to estimate their length.
Also, codegen here looks a bit complicated, I'm still learning how to use it.
I think explicit tests for sparsity patterns are missing (they're used inside other tests, but they themselves don't have a standalone test)

Manual tests:

  • Test with a single DiscontinuousLagrange
  • Test with multiple DiscontinuousLagrange
  • Test with older tests to make sure nothing breaks
  • Test with DiscontinuousLagrange and Lagrange
  • Test coupling

Tasks:

  • Write tests
  • Code works fine and doesn't break anything. I hope
  • Update docs

Things to check:

  • push! vs size precalculation
  • L2 trait
  • I think I should stop removing the extra whitespaces :)

@codecov-commenter
Copy link

codecov-commenter commented May 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.14 🎉

Comparison is base (3cff601) 91.77% compared to head (1130709) 91.92%.

❗ Current head 1130709 differs from pull request most recent head 7aaada1. Consider uploading reports for the commit 7aaada1 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #710      +/-   ##
==========================================
+ Coverage   91.77%   91.92%   +0.14%     
==========================================
  Files          32       32              
  Lines        4718     4766      +48     
==========================================
+ Hits         4330     4381      +51     
+ Misses        388      385       -3     
Impacted Files Coverage Δ
src/Grid/grid.jl 92.08% <ø> (ø)
src/Dofs/sparsity_pattern.jl 100.00% <100.00%> (+0.73%) ⬆️
src/interpolations.jl 96.75% <100.00%> (+0.21%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@fredrikekre fredrikekre added feature gsoc23 Google Summer of Code 2023 labels May 16, 2023
@fredrikekre
Copy link
Member

Nice. Didn't look at the details, but the code is surprisingly short.

It iterates over all cells twice, once to get the length of I,J and another to fill them.

Probably you can just push! then instead? Would be interesting to compare the two approaches. push! is generally pretty fast, but if you push into an already large vector it might have to be reallocated, which is expensive. That is why we currently make an estimate.

test/test_dofs.jl Outdated Show resolved Hide resolved
Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

Some comments.

src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
src/interpolations.jl Outdated Show resolved Hide resolved
src/interpolations.jl Outdated Show resolved Hide resolved
src/interpolations.jl Outdated Show resolved Hide resolved
src/interpolations.jl Outdated Show resolved Hide resolved
src/Dofs/sparsity_pattern.jl Outdated Show resolved Hide resolved
@termi-official
Copy link
Member

Can you test and clarify the case when we use discontinuous interpolations with shared dofs? (e.g. Crouzeix-Raviart)

@fredrikekre fredrikekre merged commit 5a7c081 into Ferrite-FEM:master Jul 25, 2023
6 checks passed
@AbdAlazezAhmed AbdAlazezAhmed deleted the elements-coupling branch August 12, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature gsoc23 Google Summer of Code 2023
Projects
Development

Successfully merging this pull request may close these issues.

6 participants