Vpp visualization #11274

friedmud · 2018-04-13T16:46:41Z

Fills an Aux field with the values of a num_procs length vector.

permcody · 2018-04-13T17:16:21Z

framework/doc/content/documentation/systems/AuxKernels/VectorPostprocessorVisualizationAux.md

+
+## Description
+
+This object is intended to let you view VectorPostprocessor vectors that are of lenght `num_procs` (meaning there is one value per MPI process).  This object will take those values and fill up an Auxiliary field with them so the values can be visualized.


I guess as a utility this is fine, but there are better ways to accomplish this. If you take a step back and look at these new utilities you've added in the past week, you are essentially taking local only information, replicating it on all processors, then using the local only information. We could have just never sent it in the first place if you are only using the AuxKernel, but I suppose you may want both the VPP information and the visualization field.

Ok - can you enumerate the "better ways"? And I'd like to see some citations on these new utilities you've added in the past week, you are essentially taking local only information, replicating it on all processors, then using the local only information

Where?

First of all - I didn't replicate information to all at all with any of my previous utilities. WorkBalance, HistogramVectorPostprocessor and StatisticsVectorPostprocessor only compute final values on processor zero (by default).

Besides that - HistorgramVectorPostprocessor and StatisticsVectorPostprocessor work on any VPP... not just VPPs that compute one value per processor. For instance: you can use it to compute the mean, max, sum, average or get the Histogram of values found by a LineValueSampler... including the statistics/historgram for all of the variables you sampled by that LineValueSampler. Please let me know how else you would do this.

I will agree that in this case for visualization- for WorkBalance - it needs to broadcast to get a copy of the information on each processor - but there might be other reasons for doing that besides just visualizing it. AND this utility is not only intended for WorkBalance alone... I'm going to use it to do many other things with... where I already have the vectors on each processor (like my ray tracing data).

What I'm making here are basic tools. I have many other plans for them going forward with my dissertation. If you don't find them useful then I'll just put them in my app. I know that many of these will get used a TON by many people over the years.

Also: please tell me how to get the partition surface area on each processor and paint it into a field only using an AuxKernel?

👍 resolved

permcody · 2018-04-13T17:17:53Z

test/tests/auxkernels/vector_postprocessor_visualization/vector_postprocessor_visualization.i

+  [wb]
+    type = WorkBalance
+    sync_to_all_procs = 'true'
+    execute_on = 'INITIAL'


Shouldn't this be 'TIMESTEP_END' too if you are visualizing every timestep (see above)?

Nope - the issue is a dependency resolution issue.

Oh! Then this is a bug! VectorPostprocessors should be using the same PRE/POST logic as PostProcessors (and UserObjects). I'll checkout your branch and see if I can fix this.

OK - So there isn't a bug here. Turns out that the check in VectorPostprocessorVisualizationAux::timestepSetup() is overzealous in that it runs in only one spot (e.g. at the beginning of the timestep). The only way to get around that is to run the VPP that you are trying to visualize before that, which only leaves INITIAL. However, that defeats the purpose of the check since VPPs are allowed to change the size of their vectors with each invocation.

permcody · 2018-04-13T17:17:56Z

framework/src/vectorpostprocessors/WorkBalance.C

+    _communicator.allgather(static_cast<Real>(_local_num_nodes), _num_nodes);
+    _communicator.allgather(static_cast<Real>(_local_num_dofs), _num_dofs);
+    _communicator.allgather(static_cast<Real>(_local_num_partition_sides), _num_partition_sides);
+    _communicator.allgather(_local_partition_surface_area, _partition_surface_area);


This really isn't the right idea here. You should be usingscatter instead of allgather:
https://github.com/libMesh/libmesh/blob/master/include/parallel/parallel.h#L1240-L1247

Nope. allgather() is the right one.

All processors are contributing one value that gets inserted in their rank position on every processor. That's exactly what allgather() does.

Read up: http://mpitutorial.com/tutorials/mpi-scatter-gather-and-allgather/

Just to record our conversation that we had offline here: What I was after here was gather followed by scatter, not scatter instead of allgather. However, the scatter step should really be part of the AuxKernel, not the VPP. We decided that this isn't necessary for now as it won't impede scalability until we reach at least 100K - 1M procs.

BTW - I thought of something else that could help out here. We have the UserObject::spatialValue() and associated AuxKernel that pulls those out. If we had a similar interface for VPPs, you could actually scatter within your VPP and just store the scalar that you want to dump out. This would put the logic back into the VPP instead of the AuxKernel.

I think we should consider fleshing this interface out more.

👍 resolved

permcody · 2018-04-13T17:33:45Z

test/tests/auxkernels/vector_postprocessor_visualization/vector_postprocessor_visualization.i

+    sync_to_all_procs = 'true'
+    execute_on = 'INITIAL'
+  []
+[]


Oh, You'll want to make sure to add a newline here. This isn't pedantic. It'll actually be removed if somebody runs the "remove_trailing_whitespace" script which sometimes causes confusion since it wouldn't have been edited by that developer otherwise.

Also though: this didn't need a comment because Precheck caught it...

moosebuild · 2018-04-15T01:53:12Z

Job Documentation on 60a0b9e wanted to post the following:

View the site here

This comment will be updated on new commits.

permcody · 2018-04-16T15:03:33Z

framework/src/auxkernels/VectorPostprocessorVisualizationAux.C

+VectorPostprocessorVisualizationAux::VectorPostprocessorVisualizationAux(
+    const InputParameters & parameters)
+  : AuxKernel(parameters),
+    _vpp_vector(getVectorPostprocessorValue("vpp", getParam<std::string>("vector_name"))),


Interesting that this works here. It actually isn't correct. We have all of those "ByName" variants, which take the parameter name (instead of the actual name). This might mean we have another bug in the retrieval of VPPs.

It's right the way it is - it is using the non "by name" version on purpose. vpp is the parameter name.

There is no such thing for the vector_name though (which is why I have to do getParam() for it). It could be argued that we should make that part of the interface too...

friedmud · 2018-05-02T23:49:41Z

@permcody can we merge this? It will make it easier for me to develop and test the broadcast/scatter code...

moosebuild · 2018-05-03T03:16:53Z

Job App tests on 60a0b9e : invalidated by @friedmud

permcody · 2018-05-03T14:44:05Z

@friedmud - It's approved. You can either merge it yourself or build onto it. I'm not sure anyone is jumping to use it so I'll leave that up to you.

permcody requested changes Apr 13, 2018

View reviewed changes

permcody reviewed Apr 13, 2018

View reviewed changes

friedmud added a commit to friedmud/moose that referenced this pull request Apr 14, 2018

Addressing comments refs idaholab#11274

3684cbe

friedmud force-pushed the vpp_visualization_11272 branch from 3684cbe to e85fc6a Compare April 15, 2018 01:25

friedmud added a commit to friedmud/moose that referenced this pull request Apr 15, 2018

Addressing comments refs idaholab#11274

e85fc6a

friedmud force-pushed the vpp_visualization_11272 branch from e85fc6a to 0a8b829 Compare April 15, 2018 18:20

friedmud added a commit to friedmud/moose that referenced this pull request Apr 15, 2018

Addressing comments refs idaholab#11274

0a8b829

permcody reviewed Apr 16, 2018

View reviewed changes

permcody added the PR: Address comments label Apr 16, 2018

friedmud removed the PR: Address comments label May 2, 2018

friedmud added 3 commits May 2, 2018 18:01

Add VectorPostprocessorVisualization refs idaholab#11272

13823af

Documentation for VPPV closes idaholab#11272

7e899ce

Addressing comments refs idaholab#11274

60a0b9e

friedmud force-pushed the vpp_visualization_11272 branch from 0a8b829 to 60a0b9e Compare May 3, 2018 00:02

friedmud added the PR: Ready for review/merge label May 3, 2018

permcody approved these changes May 3, 2018

View reviewed changes

friedmud merged commit 88584ee into idaholab:devel May 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vpp visualization #11274

Vpp visualization #11274

friedmud commented Apr 13, 2018

permcody Apr 13, 2018

friedmud Apr 14, 2018 •

edited

friedmud Apr 14, 2018

permcody Apr 16, 2018

permcody Apr 13, 2018

friedmud Apr 14, 2018

permcody Apr 16, 2018

permcody Apr 16, 2018

permcody Apr 13, 2018

friedmud Apr 14, 2018

permcody Apr 16, 2018 •

edited

permcody Apr 16, 2018

permcody Apr 16, 2018

permcody Apr 13, 2018

friedmud Apr 14, 2018

friedmud Apr 14, 2018

moosebuild commented Apr 15, 2018 •

edited

permcody Apr 16, 2018

friedmud Apr 16, 2018

permcody Apr 16, 2018

friedmud commented May 2, 2018

moosebuild commented May 3, 2018

permcody commented May 3, 2018


		## Description

		This object is intended to let you view VectorPostprocessor vectors that are of lenght `num_procs` (meaning there is one value per MPI process). This object will take those values and fill up an Auxiliary field with them so the values can be visualized.

Vpp visualization #11274

Vpp visualization #11274

Conversation

friedmud commented Apr 13, 2018

Choose a reason for hiding this comment

friedmud Apr 14, 2018 • edited

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

permcody Apr 16, 2018 • edited

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 Apr 15, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedmud commented May 2, 2018

moosebuild commented May 3, 2018

permcody commented May 3, 2018

friedmud Apr 14, 2018 •

edited

permcody Apr 16, 2018 •

edited

moosebuild commented Apr 15, 2018 •

edited