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

add!(::ConstraintHandler, ::Dirichlet) for nonconcrete celltypes #427

Merged
merged 8 commits into from
Jan 9, 2023

Conversation

KnutAM
Copy link
Member

@KnutAM KnutAM commented Feb 23, 2022

Allow the following syntax also for a Dirichlet condition covering nonconcrete celltypes

add!(ch, dbc)

Before, this had to be done for each fieldhandler in ch::ConstraintHandler with a unique dbc::Dirichlet for each fieldhandler.

@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2022

Codecov Report

Base: 92.92% // Head: 92.87% // Decreases project coverage by -0.04% ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #427      +/-   ##
==========================================
- Coverage   92.92%   92.87%   -0.05%     
==========================================
  Files          27       27              
  Lines        4111     4127      +16     
==========================================
+ Hits         3820     3833      +13     
- Misses        291      294       +3     
Impacted Files Coverage Δ
src/Dofs/ConstraintHandler.jl 96.00% <100.00%> (+0.06%) ⬆️
src/Dofs/MixedDofHandler.jl 86.09% <0.00%> (-1.35%) ⬇️

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 KnutAM marked this pull request as ready for review February 25, 2022 19:22
@KnutAM
Copy link
Member Author

KnutAM commented Mar 23, 2022

Could you review @kimauth?

@KnutAM
Copy link
Member Author

KnutAM commented Dec 22, 2022

The current version doesn't support the strategy from #459 (comment).
Todo: Change deepcopy to just copying the set and not the entire Dirichlet struct.
Edit: Done in ea4eaff

@KnutAM KnutAM marked this pull request as draft December 22, 2022 06:58
@@ -43,6 +43,7 @@ SHA = "ea8e919c-243c-51af-8825-aaa63cd721ce"
StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
TimerOutputs = "a759f4b9-e2f1-59dc-863e-4aeb61b1ea8f"
Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
Copy link
Member

Choose a reason for hiding this comment

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

@test_logs comes from Test so I don't think this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, must have missed that when testing (probably tried without using Test) :)
Will remove

Copy link
Member Author

Choose a reason for hiding this comment

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

Edit: It seems actually to be required for min_level=Logging.Warn

if overlaps(fh, dbc)
# If dbc have dofs not in fh, then these will be removed from dbc, hence deepcopy
# In this case, add! will warn, unless warn_check_cellset=false
add!(ch, fh, deepcopy(dbc), warn_check_cellset=false)
Copy link
Member

Choose a reason for hiding this comment

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

I believe I asked @kimauth this some time -- but why is there a need to copy at all? See also

return Dirichlet(f, copy(faces), field_name, __to_components(components), Int[], Int[])
which is what I might have asked about.

Copy link
Member Author

Choose a reason for hiding this comment

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

The deepcopy is required due to

delete!(dbc.faces, Index(cellidx, faceidx))

Copy link
Member Author

Choose a reason for hiding this comment

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

But perhaps it would be better to rather recreate the entire Dirichlet like in your link, but perhaps here:

# loop over all the faces in the set and add the global dofs to `constrained_dofs`
instead, and then getting rid of that copy upon construction too?

Copy link
Member

Choose a reason for hiding this comment

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

Yea I don't really understand why this doesn't just skip.

@KnutAM KnutAM marked this pull request as ready for review December 28, 2022 16:54
@KnutAM KnutAM requested a review from kimauth January 2, 2023 09:54
@lijas
Copy link
Collaborator

lijas commented Jan 4, 2023

I recently ran to the issue which this PR resolves (I wanted to add a constraint on a surface/faceset which had cells in different fieldhandlers). Looks good to me :)

@KnutAM KnutAM merged commit 372ab4a into Ferrite-FEM:master Jan 9, 2023
@KnutAM KnutAM deleted the kam/addmdhconstraint branch January 9, 2023 09:58
fredrikekre added a commit that referenced this pull request Apr 4, 2023
This patch cleans up the code under
```
add!(::ConstraintHandler, ::Dirichlet)
```
a bit after the DofHandler merge. In particular, it deprecates the
method
```
add!(::ConstraintHandler, ::FieldHandler, ::Dirichlet)
```
in favor of just
```
add!(::ConstraintHandler, ::Dirichlet)
```
which was added in #427. There is no need to be able to specify the
constrained set by both the set passed to `Dirichlet` and by the set
given to the `FieldHandler`. Looking at the tests modified in this
patch, it seems this was never the intention anyway. This patch also
removes the copy of the set in the `Dirichlet` constructor in favor of
explicit partitioning of the set over the `FieldHandler`s.
fredrikekre added a commit that referenced this pull request Apr 4, 2023
This patch cleans up the code for adding `Dirichlet` constraints to the
`ConstraintHandler` a bit after the DofHandler merge. In particular, it
deprecates the method
```
add!(::ConstraintHandler, ::FieldHandler, ::Dirichlet)
```
in favor of just
```
add!(::ConstraintHandler, ::Dirichlet)
```
which was added in #427. There is no need to be able to specify the
constrained set by both the set passed to `Dirichlet` and by the set
given to the `FieldHandler`. Looking at the tests modified in this
patch, it seems this was never the intention anyway. This patch also
removes the copy of the set in the `Dirichlet` constructor in favor of
explicit partitioning of the set over the `FieldHandler`s.
fredrikekre added a commit that referenced this pull request Apr 4, 2023
This patch cleans up the code for adding `Dirichlet` constraints to the
`ConstraintHandler` a bit after the DofHandler merge. In particular, it
deprecates the method
```
add!(::ConstraintHandler, ::FieldHandler, ::Dirichlet)
```
in favor of just
```
add!(::ConstraintHandler, ::Dirichlet)
```
which was added in #427. There is no need to be able to specify the
constrained set by both the set passed to `Dirichlet` and by the set
given to the `FieldHandler`. Looking at the tests modified in this
patch, it seems this was never the intention anyway. This patch also
removes the copy of the set in the `Dirichlet` constructor in favor of
explicit partitioning of the set over the `FieldHandler`s.
fredrikekre added a commit that referenced this pull request Apr 4, 2023
This patch cleans up the code for adding `Dirichlet` constraints to the
`ConstraintHandler` a bit after the DofHandler merge. In particular, it
deprecates the method
```
add!(::ConstraintHandler, ::FieldHandler, ::Dirichlet)
```
in favor of just
```
add!(::ConstraintHandler, ::Dirichlet)
```
which was added in #427. There is no need to be able to specify the
constrained set by both the set passed to `Dirichlet` and by the set
given to the `FieldHandler`. Looking at the tests modified in this
patch, it seems this was never the intention anyway. This patch also
removes the copy of the set in the `Dirichlet` constructor in favor of
explicit partitioning of the set over the `FieldHandler`s.
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.

None yet

4 participants