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.

Sign up for GitHub

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

Jump to bottom

Add nearest point average userobject #18541

Merged
merged 7 commits into from
Dec 13, 2022
Merged

Add nearest point average userobject #18541

merged 7 commits into from
Dec 13, 2022

Conversation

aprilnovak
Copy link
Contributor

Moves the NearestPointAverage userobject from Cardinal to MOOSE.

Reason

This is a general feature that I would like to use in Pronghorn as well as Cardinal.

Design

Simple copy of source code from Cardinal to MOOSE.

Impact

No expected impact.

Closes #18536

@moosebuild
Copy link
Contributor

moosebuild commented Aug 6, 2021 •
edited

Job Documentation on fdeeb0a wanted to post the following:

View the site here

This comment will be updated on new commits.

@dschwen dschwen changed the title Add nearest point average userobject. Refs #18536 Add nearest point average userobject Aug 6, 2021
@idaholab idaholab deleted a comment from moosebuild Aug 6, 2021
@aprilnovak
Copy link
Contributor Author

I'm not sure how to fix these documentation errors... I just copied the existing NearestPointLayeredAverage.md and adjusted the description and class name for the NearestPointAverage case.

@GiudGiud
Copy link
Contributor

GiudGiud commented Aug 12, 2021 •
edited

cant see anything wrong either.
I invalidated see if it helps
EDIT: it wasnt a fluke. Something somewhere must be mispelled or redundant.
Modules doc are complaining. Is it that way for moose objects usually?

@GiudGiud
Copy link
Contributor

I cannot tell you why, but I think the issue is that what you added is a VectorPostprocessor more than a UserObject.
So in the docs you should have !syntax description /VectorPostprocessors/NearestPointAverage

This is related to #18565

@aprilnovak
Copy link
Contributor Author

I think you're right... the NearestPointIntegralVariablePostprocessor class also inherits from ElementVariableVectorPostprocessor, and uses VectorPostprocessors in the documentation

@aprilnovak
Copy link
Contributor Author

nah, that didn't seem to be it, or it just part of it...

@GiudGiud
Copy link
Contributor

Made it worse. This fails moose documentation now, not just modules

@GiudGiud
Copy link
Contributor

@aeslaughter this is getting documentation errors, and I cannot figure out why. If you could please have a look

@aeslaughter
Copy link
Contributor

I found the problem, I am working on a solution. Something is getting messed up with the syntax of the object is being stored, it is being registered with syntax of Postprocessors by MOOSE. I am guessing the calls to validParams for both a Postprocessor and a VPP are overwriting something. I will push up the fix when I figure it out.

aeslaughter added a commit to aprilnovak/moose that referenced this pull request Aug 20, 2021
@aeslaughter
Maintain _moose_base parameter for BaseType in NearestPointBase
7aac50c 
(refs idaholab#18541)
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale PRs that have reached or exceeded 90 days with no activity label Mar 12, 2022
@github-actions github-actions bot closed this Mar 16, 2022
@aprilnovak
Copy link
Contributor Author

Ah, didn't catch that this went stale and forgot about it until now

@aprilnovak
Copy link
Contributor Author

@GiudGiud what is the process to re-open a stale PR? Push a fresh PR?

@GiudGiud
Copy link
Contributor

just reopen like this

@GiudGiud GiudGiud reopened this Nov 29, 2022
@aprilnovak
Copy link
Contributor Author

Either I'm missing it, or I don't have permissions to re-open closed PRs. For instance, on a separate stale PR of mine, it looks like this at the bottom of the page.

Screen Shot 2022-11-29 at 3 05 15 PM

@GiudGiud
Copy link
Contributor

ah maybe not.
Doesnt add much since you could have made another one.

@GiudGiud GiudGiud mentioned this pull request Nov 30, 2022
Closed
@GiudGiud GiudGiud removed the stale PRs that have reached or exceeded 90 days with no activity label Nov 30, 2022
@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 30, 2022 •
edited

does this commit that Andrew worked on solve the problem?
aprilnovak@7aac50c

@aprilnovak
Copy link
Contributor Author

Whoops, did I overwrite some old commits when I rebased?

In any case, whatever Andrew was working on back then did not completely solve the problem (the PR sat in a "failed" state when it went stale). I'll try re-adding that commit, but if my memory serves right it wasn't the complete fix.

@GiudGiud
Copy link
Contributor

dont think so, the commit was not part of the PR. But it s out there if it helps

@moosebuild
Copy link
Contributor

Job Precheck on e3cff45 : invalidated by @aprilnovak

@moosebuild
Copy link
Contributor

moosebuild commented Dec 10, 2022 •
edited

Job Coverage on fdeeb0a wanted to post the following:

Framework coverage

d653ad #18541 fdeeb0
Total Total +/- New
Rate 84.50% 84.49% -0.00% 75.00%
Hits 81145 81175 +30 30
Misses 14889 14899 +10 10

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

Warnings

  • framework new line coverage rate 75.00% is less than the suggested 90.0%

This comment will be updated on new commits.

@GiudGiud
Register the nearest point average to a UO base. The two bases it cou…
fdeeb0a 
…ld have inherited from its template arguments are PP and VPP, and this object is meant to be a UO

Refs idaholab#18536
@GiudGiud
Copy link
Contributor

builds docs locally, fingers crossed

GiudGiud
GiudGiud approved these changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

@cticenhour I added a commit can you please review that one

@aprilnovak
Copy link
Contributor Author

Thanks @GiudGiud!

cticenhour
cticenhour approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@cticenhour cticenhour left a comment

Choose a reason for hiding this comment

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

Approving @GiudGiud's commit fdeeb0a

@cticenhour
Copy link
Member

@aprilnovak Would you consider adding a news entry as a follow-up to this PR? Since we're ready to merge on this, I won't hold things up, but a brief summary of the object and what it adds to MOOSE would be appreciated for other users!

@aprilnovak
Copy link
Contributor Author

@cticenhour of course! I'll add one as soon as this is merged so that I can cross-reference the object from the other PR

@GiudGiud GiudGiud merged commit 0890e95 into idaholab:next Dec 13, 2022
@aprilnovak aprilnovak mentioned this pull request Jan 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cticenhour cticenhour cticenhour approved these changes

@GiudGiud GiudGiud GiudGiud approved these changes

@aeslaughter aeslaughter Awaiting requested review from aeslaughter

Assignees

@GiudGiud GiudGiud

Labels
PR: Failed but allowed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add userobject to compute nearest point averages
5 participants
@aprilnovak @moosebuild @GiudGiud @aeslaughter @cticenhour