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

make flag to optionally print/suppress aux variable creation printing #29665

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

tophmatthews
Copy link
Contributor

Closes #29664

@moosebuild
Copy link
Contributor

moosebuild commented Jan 9, 2025

Job Documentation, step Docs: sync website on 960e9a4 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Jan 9, 2025

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

Framework coverage

e193a3 #29665 960e9a
Total Total +/- New
Rate 85.25% 85.25% +0.00% 100.00%
Hits 108024 108027 +3 4
Misses 18686 18686 - 0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

const Action * common = common_actions.empty() ? nullptr : *common_actions.begin();
if (common && !common->getParam<bool>("print_automatic_aux_variable_creation"))
print_aux_creation = false;
if (material_names.size() > 0 && print_aux_creation)
Copy link
Contributor

Choose a reason for hiding this comment

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

so you are really adding this to silence it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Maybe there's a better way, but I wanted to protect against common not being found (or at least copy and pasted it from the way it's done elsewhere). In otherwords, if common is not found, allow it to print as normal.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean the PR in general not just these few lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ha! Yes, I'll change it...

@GiudGiud GiudGiud self-assigned this Jan 9, 2025
@tophmatthews tophmatthews changed the title make flag to print aux variable creation make flag to optionally print/suppress aux variable creation printing Jan 9, 2025
@GiudGiud
Copy link
Contributor

GiudGiud commented Jan 9, 2025

I ll add a commit soon as I was tying something out. I think CommonOuptutAction isnt the best fit here, but with some changes MaterialOutputAction can host the capability / option on its own.

@GiudGiud GiudGiud force-pushed the mat_aux_output_29664 branch from ff0159a to 45ea0f2 Compare January 9, 2025 17:08
@GiudGiud
Copy link
Contributor

GiudGiud commented Jan 9, 2025

@lindsayad for the independent review please

@tophmatthews
Copy link
Contributor Author

slick! Thanks!

@tophmatthews
Copy link
Contributor Author

tophmatthews commented Jan 9, 2025

The griffin failures look non-related.

@GiudGiud
Copy link
Contributor

GiudGiud commented Jan 9, 2025

I m not sure. Do they happen elsewhere? They seem tied to material properties output in an exodus file

@tophmatthews
Copy link
Contributor Author

Yeah, they don't happen in another PR

@tophmatthews
Copy link
Contributor Author

exodiff: DIFFERENCE .. The element variable "heating_factor_vec_g0" is in the first file but not the second.

@GiudGiud
Copy link
Contributor

GiudGiud commented Jan 9, 2025

I'm building griffin to take a look

@tophmatthews
Copy link
Contributor Author

@GiudGiud Did you figure out the griffin problem? The doco problem is simple. I can fix it, but didn't want to get tangled with your updates.

@GiudGiud
Copy link
Contributor

GiudGiud commented Jan 9, 2025

No not yet. It's got to do with how they inherit the action in one of their own actions but no luck yet

@GiudGiud GiudGiud force-pushed the mat_aux_output_29664 branch from fe27f13 to 960e9a4 Compare January 10, 2025 13:07
@GiudGiud
Copy link
Contributor

doco should be good (though I ll watch it)
Griffin patch is here if you have access https://github.inl.gov/ncrc/griffin/pull/2292

@GiudGiud
Copy link
Contributor

Thanks for the review Alex! I'll re-request if we make any significant change but I dont think we will need to

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.

Suppress printing of automatic aux variable creation
4 participants