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

Introduce getgrid(dh::AbstractDofHandler) function #552

Closed
wants to merge 2 commits into from
Closed

Conversation

KnutAM
Copy link
Member

@KnutAM KnutAM commented Dec 12, 2022

I find myself quite often using dh.grid, and that is also the case in Ferrite's source code.
While this is fine for both DofHandler and MixedDofHandler, it feels "dangerous" for AbstractDofHandler, and I think it would be nice to have an access function here.

If you think this is a good idea, like [and subscribe]:)
Then I will go through the rest of the source code and update it.
For now, only DofHandler.jl is updated as a demo

@fredrikekre
Copy link
Member

I am at least a sucker for field access, and disliker of get* functions :)

@termi-official
Copy link
Member

Note: This is related to #486 . I propagated getgrid there through most of the code as a compatibility layer against non-distributed assembly.

@KnutAM
Copy link
Member Author

KnutAM commented Dec 12, 2022

Note: This is related to #486 . I propagated getgrid there through most of the code as a compatibility layer against non-distributed assembly.

Ah, great 👍 - I missed that you had already done that there!
What's the status/plan with that one?
Will that be merged as a whole, or should such small changes be pulled out as separate prs and merged individually?

@termi-official
Copy link
Member

No problem. I have to admit that the PR is really hard to read, which is why I am trying to break the PR into several smaller ones, fixing one open issue after another. Two major things which I want to consider first is

  1. propagating the abstraction layer through the codebase and addressing some coupling issues regarding mixed-dimensional and mixed-element meshes (see Decoupling of Geometrical, Topological and Algebraical Interfaces #519)
  2. merge the dof handlers, because otherwise I have to write two separate dof handlers for the distributed assembly, or possibly a third, incompatible one, none of which is satisfactory for me.

However, since I am currently not having time to address these issues properly, I am fine if you cherry pick some changes, polish them, and merge them.

@codecov-commenter
Copy link

Codecov Report

Base: 92.76% // Head: 92.81% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (90175cb) compared to base (f35b778).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 90175cb differs from pull request most recent head e7873d2. Consider uploading reports for the commit e7873d2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #552      +/-   ##
==========================================
+ Coverage   92.76%   92.81%   +0.04%     
==========================================
  Files          28       24       -4     
  Lines        4203     3979     -224     
==========================================
- Hits         3899     3693     -206     
+ Misses        304      286      -18     
Impacted Files Coverage Δ
src/Dofs/DofHandler.jl 91.06% <100.00%> (+0.60%) ⬆️
src/utils.jl 36.36% <0.00%> (-9.80%) ⬇️
src/arrayutils.jl 60.00% <0.00%> (-9.05%) ⬇️
src/interpolations.jl 90.00% <0.00%> (-1.49%) ⬇️
src/Quadrature/quadrature.jl 94.23% <0.00%> (-0.32%) ⬇️
src/Quadrature/generate_quadrature.jl 67.17% <0.00%> (-0.25%) ⬇️
src/Grid/grid.jl 87.30% <0.00%> (-0.09%) ⬇️
src/Dofs/ConstraintHandler.jl 95.92% <0.00%> (-0.08%) ⬇️
src/L2_projection.jl 100.00% <0.00%> (ø)
src/FEValues/common_values.jl 92.75% <0.00%> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@KnutAM
Copy link
Member Author

KnutAM commented Feb 17, 2023

Closing as handled in #486

@KnutAM KnutAM closed this Feb 17, 2023
@KnutAM KnutAM deleted the kam/getgrid branch April 3, 2023 20:15
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.

4 participants