Cleanup of Materials output system #29820

shikhar413 · 2025-02-06T14:47:30Z

Reason

Design

Impact

refs #29816, #29787, #29781, #29780, and #29819

- Fix faulty logic when materials are outputted to different files - Make sure setting outputs = 'all none' results in an error - Confirm that outputs = "exodus", outputs = "all", and outputs = "none" work as expected

GiudGiud

I dont know if the MaterialPropertyOutput works as expected with VTK/Nemsis, but even if it does not we dont want to further make it only work with exodus

GiudGiud · 2025-02-06T16:56:06Z

framework/doc/content/syntax/Materials/index.md

@@ -207,7 +207,14 @@ vector or tensor value.
 ## Material Property Output

 Output of `Material` properties is enabled by setting the "outputs" parameter. The following example
-creates two additional variables called "mat1" and "mat2" that will show up in the output file.
+creates two additional variables called "mat1" and "mat2" that will show up in the output file. In this
+example, the `exodus` name is a special keyword used to signal to MOOSE that the material properties


is exodus output is the only output handling material properties?
what about Nemesis? VTK ?
The material property output goes to these too

That’s a good point, I’m not sure let me check

GiudGiud · 2025-02-06T17:01:37Z

framework/doc/content/syntax/Materials/index.md

+creates two additional variables called "mat1" and "mat2" that will show up in the output file. In this
+example, the `exodus` name is a special keyword used to signal to MOOSE that the material properties
+should be outputted to the output object created when setting `Outputs/exodus=true`. If multiple output
+Exodus objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"


Suggested change

Exodus objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"

[Exodus.md] objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"

GiudGiud · 2025-02-06T17:02:23Z

framework/doc/content/syntax/Materials/index.md

+should be outputted to the output object created when setting `Outputs/exodus=true`. If multiple output
+Exodus objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"
+parameter. In addition, the reserved output name `all` can be used to output the material property to
+all Exodus objects in the `[Outputs]` block, while the reserved output name `none` can be used to


split the text between the example showing an exodus case and 'all'/'none' applying beyond just exodus

GiudGiud · 2025-02-06T17:03:24Z

framework/include/outputs/OutputWarehouse.h

+   * @param is_exodus Optional parameter to check if the output object associated with name must be
+   *                  Exodus type


why are we targetting exodus? Let's find a templated way

GiudGiud · 2025-02-06T17:04:31Z

framework/src/actions/CheckOutputAction.C

-    // Check that the outputs exist
-    _app.getOutputWarehouse().checkOutputs(outputs);
+    // Check that the outputs exist, and that they are of Exodus type
+    _app.getOutputWarehouse().checkOutputs(outputs, /* exodus = */ true);


but we could want to be outputting to nemesis / VTK outputs

GiudGiud · 2025-02-06T17:06:20Z

framework/src/outputs/OutputWarehouse.C

@@ -120,9 +120,18 @@ OutputWarehouse::addOutput(std::shared_ptr<Output> const output)
 }

 bool
-OutputWarehouse::hasOutput(const std::string & name) const
+OutputWarehouse::hasOutput(const std::string & name, const bool is_exodus) const


I d rather you add a templated version over this boolean

test/tests/materials/output/gold/output_multiple_files_exodus1.e

GiudGiud · 2025-02-06T17:08:28Z

test/tests/materials/output/tests

+                  Outputs/exodus2/file_base='output_exodus2_mat_block2'"
+      detail = 'outputting certain properties within the material definition to different files;'
+    []
+    [multiple_materials_single_file]


these tests are great

moosebuild · 2025-02-06T19:00:47Z

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

Framework coverage

	93913d	#29820 8c8a58
	Total	Total	+/-	New
Rate	85.28%	85.29%	+0.01%	100.00%
Hits	108338	108366	+28	34
Misses	18697	18694	-3	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.

moosebuild · 2025-02-06T19:19:58Z

Job Documentation, step Docs: sync website on 8c8a581 wanted to post the following:

View the site here

This comment will be updated on new commits.

… output files and expected error messages with outputs parameter

…daholab#29787, idaholab#29781, idaholab#29780, idaholab#29819

shikhar413 · 2025-02-07T18:02:59Z

I dont know if the MaterialPropertyOutput works as expected with VTK/Nemsis, but even if it does not we dont want to further make it only work with exodus

@GiudGiud I'm looking into this a bit more. I did confirm that materials output is supported on other data formats other than Exodus, here are the ones I tested that do have material properties outputted, though I'm not sure all of these have unit tests:
exodus, vtk, nemesis, tecplot, xda, xdr

Then, there are other output formats that have no effect on material output, such as console, csv, and gnuplot

And then there are others that I don't know if material property output is doing anything, since I don't know how to confirm this for the particular output format - gmv and checkpoint

Basically, the materials output system will define an AuxKernel that stores material data, and if the output format is able to parse this AuxKernel data into the output data format, then the material output functionality is supported for that data format

I like the option of templating the functionality of checking whether the output object meets a certain list of types. However, I'm not sure if this is possible without hardcoding the datatypes where this is supported, since some data types will output the material output data, and others will just ignore this data. Not to mention that some other modules/apps have their own custom output format that might be able to support material outputting as well. Right now I'm leaning towards removing the requirement for the output name to be of a certain type, what do you think?

GiudGiud · 2025-02-07T19:31:56Z

Does the selection of which properties to output rely on the hide/show parameters (and subsequent attributes) of AdvancedOutput by any chance?
This output base class is widely used and could be a filter on which output to support

if this is possible without hardcoding the datatypes where this is supported,

I expect we'd need to hardcode something like "outputs = exodus" but something like outputs = 'some_name' or 'all' shouldnt need to?

then the material output functionality is supported for that data format

there should be some mechanism to further restrict these auxvariables, so they dont appear in every output when specified to only be output in one

shikhar413 · 2025-02-07T20:41:13Z

Does the selection of which properties to output rely on the hide/show parameters (and subsequent attributes) of AdvancedOutput by any chance?

I'm not seeing any special logic in MaterialOutputAction that is linked to AdvancedOutput. The way I understand the code, the aux variable is created regardless of the type of output object linked to the material, and then at the point of outputting aux variables to the output file, the function addInterfaceHideVariables is called to prevent the variable from being outputted if the material isn't linked to that specific output name

shikhar413 · 2025-02-07T20:46:09Z

Something I'm also noticing in the public app tests is that a lot of Malamute tests are failing because material outputs are being used on a csv output object. We should confirm that Materials output indeed doesn't do anything for csv output object and remove Materials/*/output = csv from any input files that try to do this.

shikhar413 · 2025-02-07T21:03:40Z

there should be some mechanism to further restrict these auxvariables, so they dont appear in every output when specified to only be output in one

I'm looking at the output types that inherit AdvancedOutput and it looks like most output types inherit this class, so trying to restrict the name of outputs to be of type AdvancedOutput won't be restricting the allowable output formats all that much

shikhar413 requested review from lindsayad and roystgnr as code owners February 6, 2025 14:47

General cleanup of Material Output system

bca4ddc

- Fix faulty logic when materials are outputted to different files - Make sure setting outputs = 'all none' results in an error - Confirm that outputs = "exodus", outputs = "all", and outputs = "none" work as expected

shikhar413 force-pushed the material_output_cleanup branch from 405c97f to b9b857f Compare February 6, 2025 15:46

GiudGiud requested changes Feb 6, 2025

View reviewed changes

GiudGiud self-assigned this Feb 6, 2025

shikhar413 added 2 commits February 7, 2025 12:29

Add unit tests for checking expected behavior in presence of multiple…

53e8f13

… output files and expected error messages with outputs parameter

Update documentation on Materials output system, refs idaholab#29816, i…

8c8a581

…daholab#29787, idaholab#29781, idaholab#29780, idaholab#29819

shikhar413 force-pushed the material_output_cleanup branch from b9b857f to 8c8a581 Compare February 7, 2025 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup of Materials output system #29820

Cleanup of Materials output system #29820

shikhar413 commented Feb 6, 2025 •

edited

Loading

GiudGiud left a comment

GiudGiud Feb 6, 2025

shikhar413 Feb 6, 2025

GiudGiud Feb 6, 2025

GiudGiud Feb 6, 2025

GiudGiud Feb 6, 2025

GiudGiud Feb 6, 2025

GiudGiud Feb 6, 2025

GiudGiud Feb 6, 2025

moosebuild commented Feb 6, 2025 •

edited

Loading

moosebuild commented Feb 6, 2025 •

edited

Loading

shikhar413 commented Feb 7, 2025

GiudGiud commented Feb 7, 2025 •

edited

Loading

shikhar413 commented Feb 7, 2025

shikhar413 commented Feb 7, 2025 •

edited

Loading

shikhar413 commented Feb 7, 2025

	Exodus objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"
	[Exodus.md] objects exist in the `[Outputs]` block, one or more names can be provided to the "outputs"

		* @param is_exodus Optional parameter to check if the output object associated with name must be
		* Exodus type

Cleanup of Materials output system #29820

Are you sure you want to change the base?

Cleanup of Materials output system #29820

Conversation

shikhar413 commented Feb 6, 2025 • edited Loading

Reason

Design

Impact

GiudGiud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moosebuild commented Feb 6, 2025 • edited Loading

Framework coverage

Modules coverage

Full coverage reports

moosebuild commented Feb 6, 2025 • edited Loading

shikhar413 commented Feb 7, 2025

GiudGiud commented Feb 7, 2025 • edited Loading

shikhar413 commented Feb 7, 2025

shikhar413 commented Feb 7, 2025 • edited Loading

shikhar413 commented Feb 7, 2025

shikhar413 commented Feb 6, 2025 •

edited

Loading

moosebuild commented Feb 6, 2025 •

edited

Loading

moosebuild commented Feb 6, 2025 •

edited

Loading

GiudGiud commented Feb 7, 2025 •

edited

Loading

shikhar413 commented Feb 7, 2025 •

edited

Loading