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

reporter for nodal variable values at the elements located by nearestElemsToLine #109

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

Conversation

lynnmunday
Copy link
Collaborator

Also renamed the variable used to for filtering elements and updated tests. closes #108

@moosebuild
Copy link

moosebuild commented Aug 24, 2022

Job Test on 9808a44 wanted to post the following:

View the site here

This comment will be updated on new commits.

@lynnmunday lynnmunday force-pushed the closestElemNodalVar branch from de68534 to 8a0a68e Compare August 24, 2022 16:54
…ed by the nearestElemsToLine. Also renamed the variable used to for filtering elements and updated tests. closes idaholab#108
@lynnmunday lynnmunday force-pushed the closestElemNodalVar branch from 8a0a68e to 3e7a13d Compare August 24, 2022 20:14
{
//_value should be zero on every proc so this will just communicate it.
for (size_t i = 0; i < _eid.size(); ++i)
gatherSum(_value[i]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dschwen is this a good way to get the value to all the processors?

n++;
}
if (n > 0)
value /= (Real)n;
Copy link
Member

Choose a reason for hiding this comment

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

C style cast

Copy link
Member

Choose a reason for hiding this comment

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

Real(n) or static_cast<Real>(n)

Copy link
Member

@dschwen dschwen Aug 24, 2022

Choose a reason for hiding this comment

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

but neither is needed here, just do value /= n;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dschwen would you look at this pr again. I didn't do averaging over gauss points because reporters don't have a base class that derives off the elem or nodal userobjects.

@lynnmunday lynnmunday force-pushed the closestElemNodalVar branch from 65523a0 to dd1d4b2 Compare August 25, 2022 04:29
…ass was changed because this will now work for element variables
@lynnmunday lynnmunday force-pushed the closestElemNodalVar branch from dd1d4b2 to d2f302c Compare August 25, 2022 16:27
@moosebuild
Copy link

Job Coverage on d2f302c wanted to post the following:

Coverage

c9cb25 #109 d2f302
Total Total +/- New
Rate 86.94% 87.33% +0.40% 100.00%
Hits 1138 1172 +34 35
Misses 171 170 -1 0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@rpodgorney
Copy link
Collaborator

@lynnmunday -- is this something others need? If no, it can wait until 10/1 to fix

@lynnmunday
Copy link
Collaborator Author

@lynnmunday -- is this something others need? If no, it can wait until 10/1 to fix

@rpodgorney No else needs this right now. I'll fix it in a few weeks.

@rpodgorney
Copy link
Collaborator

rpodgorney commented Aug 9, 2023

@lynnmunday what's the status of this PR? Are we waiting until next FY to finalize?

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.

closestElemToLine reporter that can sample nodal variables
4 participants