-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Default block restriction #30300
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
base: next
Are you sure you want to change the base?
Default block restriction #30300
Conversation
… (`modules/heat_transfer/test/tests/default_block_check`) (c) material_coverage_check fixing
f3f9abc
to
391abd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Some minor comments.
modules/heat_transfer/test/tests/default_block_check/convection-with-null-material.i
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need some markdown documentation for this. It will help me understand the context more. I always want to carefully understand the need any time we're adding a parameter to our kitchen sink object (FEProblemBase
)
Your PR description is nice and thorough, so a good start candidate for translation into markdown |
Most importantly, thank you for contributing! |
…d` to explain the `default_block` effect in coverage checking and block restrictions. (b) move the tests into `/test/tests/default_block_check`. (c) Fix according to Gary's comments.
Dear @hugary1995 and @lindsayad, thanks for your code review. I fixed the code according to Gary's comments and prepared some markdown documentation, adding them into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few more minor comments then it's all clear from my side.
…aultBlockConsistency()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve. We should have at least one approve from the framework team. @lindsayad
…w default_block implementation.
Job Documentation, step Docs: sync website on ec5a868 wanted to post the following: View the site here This comment will be updated on new commits. |
Job Coverage, step Generate coverage on ec5a868 wanted to post the following: Framework coverageInconsistent report tags were found between the head and base reports. Inconsistent tags: Modules coverageFluid properties
Full coverage reportsReports
This comment will be updated on new commits. |
this behavior by explicitly specifying the `block` parameter within each object. If an explicitly | ||
specified `block` is not a subset of the `default_block`, MOOSE will throw a warning | ||
to notify the user about the inconsistency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we want this to be a warning. I think one possible use case for this @hugary1995 mentioned is excluding lower-dimensional subdomains. But in that case, any lagrange multipliers rightly living on the lower-d subdomains will trigger this warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Also semantically it isn't a mistake to override default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear @lindsayad and @hugary1995, thanks for your feedback. I removed the warning and associated function and test in the new commit. Thanks!
…te the associated function and regression test.
In the new commit, I tried to address: (a) The system shall throw an error if the user specifies either Thanks for the feedback! |
…nel_coverage_check` or `material_coverage_check` together with `default_block`. (b) The system shall allow users to include `ANY_BLOCK_ID` in the `default_block` parameter. (c) Avoiding unnecessary copying of the `block_param` vector by using a reference.
a3e4877
to
c86b71a
Compare
Dear @lindsayad, I fixed the codes based on your feedback. Thanks. With the discussions with @hugary1995 and his suggestions. We tried to have the below logic as well (which is aligned with your feedback): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
…d be devoid of implementation details (test-driven development)
Dear @lindsayad, Thanks for your constructive and detailed feedback. I have fixed the codes. |
Job Conda build (ARM Mac) on ec5a868 : invalidated by @lindsayad |
Let's wait for @bwspenc 's review. |
close #30299
Reason
When using element subdomain modifiers (e.g., additive manufacturing, welding), simulations are often restricted to a specific subset of blocks (subdomains). However, users currently need to manually specify these block restrictions repeatedly in multiple places within the input file, such as
kernel_coverage_block_list
,material_coverage_block_list
, and theblock
parameter for individual Kernels, Materials, and Variables.This repetitive setup increases the chances of user error and makes input files harder to maintain. A more user-friendly and centralized way to control block restrictions is desired.
Design
This PR introduces a new parameter
default_block
under the[Problem]
block to provide a centralized way to control block restrictions throughout the input file.Setting default_block will automatically:
(a) Set
kernel_coverage_check = ONLY_LIST
andkernel_coverage_block_list = '0 1 3'
to[Problem]
.(b) Set
material_coverage_check = ONLY_LIST
andmaterial_coverage_block_list = '0 1 3'
to[Problem]
.(c) Apply the specified blocks to any BlockRestrictable object (e.g., Kernels, Materials, Variables) that does not explicitly define its own block parameter.
Flexibility & Overriding Behavior
(a) Users can still override
kernel_coverage_block_list
ormaterial_coverage_block_list
inside[Problem]
even ifdefault_block
is set.(b) Users can explicitly set block for individual Kernels, Materials, or Variables, provided that the specified blocks are a subset of default_block.
(c) If a user tries to specify blocks outside of default_block, an warning will be raised to prevent unintended behavior.
Example:
This will automatically apply to blocks
0 1 3
.But if user set:
Will show a warning:
Impact
This change improves usability and reduces duplication in input file setup, especially for simulations involving subdomain-based modifiers. It provides a clear and centralized control for block restrictions while preserving flexibility for advanced users.
This enhancement is particularly useful for applications like additive manufacturing, and welding.