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

OpenMCCellAverageProblem updates #907

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

meltawila
Copy link
Contributor

Update OpenMCCellAverageProblem to use the displaced problem mesh

(Ref. #906)

@moosebuild
Copy link
Collaborator

moosebuild commented Jun 13, 2024

Job Documentation on 17bf3d3 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Collaborator

moosebuild commented Jun 13, 2024

Job Coverage on 17bf3d3 wanted to post the following:

Coverage

b645e2 #907 17bf3d
Total Total +/- New
Rate 93.52% 93.50% -0.02% 92.45%
Hits 7269 7288 +19 49
Misses 504 507 +3 4

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@moosebuild
Copy link
Collaborator

Job Precheck on 92642be : invalidated by @meltawila

@moosebuild
Copy link
Collaborator

Job Precheck on 92642be wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/cardinal/docs/PRs/907/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format b645e25720385981abd4a4b712b9271c1dc8fef6

@moosebuild
Copy link
Collaborator

Job Apptainer build on 17bf3d3 : invalidated by @meltawila

@aprilnovak
Copy link
Collaborator

@meltawila can you please rebase this? There's been a lot of changes to OpenMCCellAverageProblem recently

@moosebuild
Copy link
Collaborator

moosebuild commented Oct 20, 2024

Job Documentation, step Sync to remote on dd2b9df wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Collaborator

moosebuild commented Oct 20, 2024

Job Coverage, step Generate coverage on dd2b9df wanted to post the following:

Coverage

85b10a #907 dd2b9d
Total Total +/- New
Rate 93.21% 93.11% -0.10% 86.17%
Hits 7644 7687 +43 81
Misses 557 569 +12 13

Diff coverage report

Full coverage report

Warnings

  • New new line coverage rate 86.17% is less than the suggested 90.0%

This comment will be updated on new commits.

@meltawila
Copy link
Contributor Author

@aprilnovak would you check this?

@@ -164,7 +165,9 @@ OpenMCCellAverageProblem::validParams()
params.addParam<int>("first_iteration_particles",
"Number of particles to use for first iteration "
"when using Dufek-Gudowski relaxation");

params.addParam<bool>("use_displaced_mesh",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nuclearkevin does this need to be modified at all to be consistent with what you did with fixed_mesh being auto-detected?

@meltawila can you please also remind me, was there a use case where we wanted to disable the displaced mesh updates in OpenMC, even if the mesh is actually moving? Or was this user option only existing because we didn't know how to detect it automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the logic used in MOOSE solid mechanics, to be able to set use_displaced_mesh = false for certain post processors/user objects only for example

Copy link
Contributor

Choose a reason for hiding this comment

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

@aprilnovak At the moment this doesn't need to be modified since the changes made to auto-detect fixed_mesh are coming in the AMR PR (#964). If this gets merged first, I can handle the necessary changes in #964. If the AMR PR gets merged ahead of this one, a single line of code will need to be added to this PR to get auto-detection working:
https://github.com/neams-th-coe/cardinal/pull/964/files#diff-7321f62ca7502f529d9917464fed4643c5e57c97c27d295f78db837d1a35a366R219

Copy link
Contributor

@nuclearkevin nuclearkevin Oct 20, 2024

Choose a reason for hiding this comment

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

For reference @meltawila this is what's necessary to automatically detect if the mesh is moving and setup the OpenMCCellAverageProblem accordingly. This include is required:

#include "CreateDisplacedProblemAction.h"

and this needs to get added to the constructor of OpenMCCellAverageProblem:

// Check to see if a displaced problem is being initialized.
const auto & dis_actions = getMooseApp().actionWarehouse().getActions<CreateDisplacedProblemAction>();
for (const auto & act : dis_actions)
{
  auto has_displaced = act->isParamValid("displacements") && act->getParam<bool>("use_displaced_mesh");
  _need_to_reinit_coupling |= (has_displaced && _use_displaced_mesh);
}

I already have this in a separate PR, so if yours gets merged first you won't need to add this.

@aprilnovak
Copy link
Collaborator

Thanks for updating the PR! We'll want to have a few tests before merging this. How about something like a CSG cylinder in OpenMC, and a cube mesh which translates around in space over and beyond the cylinder? We can visualize the CellIDAux to see the "shadow" of the CSG on the moving mesh.

@meltawila
Copy link
Contributor Author

We have test/tests/userobjects/moab_skinner/expanding-cube which I wanted to extend to use OpenMCCellAverageProblem as the test for this. I don't think CSG will work, right? either way we need to conserve the mass and update material densities with mesh movements as well

@aprilnovak
Copy link
Collaborator

The mesh motion aspect doesn't necessarily need to be interpreted as the OpenMC geometry changing. Although it's sort of weird why anyone would intentionally do this, the test I proposed would just check that the cell <--> element mapping is happening correctly.

@moosebuild
Copy link
Collaborator

Job Precheck, step Clang format on 84329ed wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/cardinal/docs/PRs/907/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 85b10ab66f4b22a8c5a3df8e69886ccdcbb53009

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