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

Unify element activation, element death and moving interface #17101

Merged
merged 2 commits into from
Mar 17, 2021

Conversation

hugary1995
Copy link
Contributor

Generalize the existing ActivateElementUserObjects, add ElementSubdomainModifiers, and fix some parallel bugs.

close #17100

@moosebuild
Copy link
Contributor

moosebuild commented Feb 23, 2021

Job Documentation on 8042a98 wanted to post the following:

View the site here

This comment will be updated on new commits.

@hugary1995
Copy link
Contributor Author

@fdkong @dschwen Could you please review this since you reviewed the original #16008 PR about element activation?

The new UO changes some of the logic (and names of several parameters, variables, etc) in the original UO so that element activation can be generalized to element death and moving interface.

The original regression tests didn't capture several parallel nodeset/sideset generation issues. I fixed those and added more regression tests.

The documentation should be helpful. It also explicitly mentions what we can do with this UO.

This PR will allow me to couple the element subdomain modifier with XFEM, to get smooth boundary/interface instead of the zig-zag boudaries. @bwspenc the XFEM element subdomain modifier will have to wait for this one to be merged first.

dewenyushu
dewenyushu previously approved these changes Feb 23, 2021
Copy link
Contributor

@dewenyushu dewenyushu left a comment

Choose a reason for hiding this comment

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

Looks great!

This PR is a better and generalized version of ActivateElementsUserObjectBase. Maybe we can later convert the tests from the old element activation code and combine these two.

Copy link
Contributor

@fdkong fdkong left a comment

Choose a reason for hiding this comment

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

Looks very good for me.

framework/include/userobject/ElementSubdomainModifier.h Outdated Show resolved Hide resolved
framework/src/userobject/ElementSubdomainModifier.C Outdated Show resolved Hide resolved
framework/src/userobject/ElementSubdomainModifier.C Outdated Show resolved Hide resolved
framework/src/userobject/ElementSubdomainModifier.C Outdated Show resolved Hide resolved
framework/src/userobject/ElementSubdomainModifier.C Outdated Show resolved Hide resolved
@dschwen
Copy link
Member

dschwen commented Mar 9, 2021

What's the deal with the empty jpg files added in this PR?

@hugary1995
Copy link
Contributor Author

What's the deal with the empty jpg files added in this PR?

I don't know. The images are actually not empty if you check them out. I guess it's a github issue?

@hugary1995
Copy link
Contributor Author

@fdkong could you take another look at it? When I said "there is no time pressure on this" I wasn't expecting this to take two weeks. I have a report due in April. Thanks.

Generalize the existing ActivateElementUserObjects, add ElementSubdomainModifiers, and fix some parallel bugs.

close idaholab#17100
@hugary1995
Copy link
Contributor Author

@fdkong I followed your suggestions and added a regression test for the steady case.

Copy link
Contributor

@fdkong fdkong left a comment

Choose a reason for hiding this comment

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

Looks great

@fdkong fdkong added the PR: Auto Merge Add this label to have CIVET merge on success label Mar 15, 2021
@fdkong fdkong merged commit 04e21cc into idaholab:next Mar 17, 2021
@loganharbour
Copy link
Member

@hugary1995 this is failing rather painfully almost everywhere in parallel: https://civet.inl.gov/event/65489/

Are you able to look into this asap or do we need to?

@hugary1995
Copy link
Contributor Author

Sure! I'll look at it ASAP!

@hugary1995
Copy link
Contributor Author

@loganharbour I can't figure it out any time soon. Can we revert it?

@loganharbour
Copy link
Member

I'll take a quick look and revert if I can't get it quickly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Auto Merge Add this label to have CIVET merge on success PR: Failed but allowed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify element activation, element death and moving interface
6 participants