idaholab / moose

Multiphysics Object Oriented Simulation Environment
https://www.mooseframework.org
GNU Lesser General Public License v2.1
1.7k stars 1.04k forks source link

Invalid output with Exodus `output_on=nonlinear` and steady executioner #4573

Closed dasmy closed 9 years ago

dasmy commented 9 years ago

Apparently the output is performed (can be seen with sequence=true), but all produced files/steps contain identical physical time information. This might be formally correct but disallows ParaView from showing them individually.

This only seems to apply to Steady Executioner because then dt==0.

A possible patch might read (I did not run the testsuite, yet)

diff --git a/framework/src/outputs/PetscOutput.C b/framework/src/outputs/PetscOutput.C
index 90b0ebc..12a78cf 100644
--- a/framework/src/outputs/PetscOutput.C
+++ b/framework/src/outputs/PetscOutput.C
@@ -113,7 +113,11 @@ PetscOutput::timestepSetupInternal()

   // Update the pseudo times
   _nonlinear_time = _time_old;                   // non-linear time starts with the previous time step
-  _nonlinear_dt = _dt/_nonlinear_dt_divisor;     // set the pseudo non-linear timestep
+  if (_dt != 0) {
+    _nonlinear_dt = _dt/_nonlinear_dt_divisor;     // set the pseudo non-linear timestep as fraction of real timestep for transient executioners
+  } else {
+    _nonlinear_dt = 1./_nonlinear_dt_divisor;     // set the pseudo non-linear timestep for steady executioners
+  }
   _linear_dt = _nonlinear_dt/_linear_dt_divisor; // set the pseudo linear timestep

   // Set the PETSc monitor functions

This works well when sequence=false because then the timesteps are assigned different pseudo-times. Thus, the intermediate steps of the nonlinear solver can be visualised in ParaView as a time series even for steady solvers.

However, if sequence=true, it still does not work. I assume the reason for this is the fact that exodus_num is reset to 1 whenever a new Exodus file is started (see Exodus::outputSetup(), line 133 in Exodus.C). Accordingly, from the visualisation point all different files still belong to the same timestep. I tried to avoid resetting exodus_num, but this breaks the exodus output: It fails with

[0] ../src/mesh/exodusII_io_helper.C, line 1704, compiled Dec 16 2014 at 08:26:17
Error writing nodal values.

Do you maybe have an idea on fixing this issue?

I could also live with the current state of the patch (i.e. it works if sequential=false and does not become worse if sequential=true and thus is an improvement). If we agree on some approach, I can prepare a pull request for this.

friedmud commented 9 years ago

@aeslaughter Can you comment on whether this is acceptable? If so... @dasmy would you mind submitting a Pull Request for this?

Thanks!

aeslaughter commented 9 years ago

I think that this type of numbering would be fine. @dasmy if you create a pull request I will take a closer look and fix up the error that you are still getting.

Also, could you add a simple test for the desired behavior. For example, utilize simple_diffusion.i but enable sequence=true and set output_on='initial timestep_end nonlinear'. This should produce a whole series of files. Thanks.

dasmy commented 9 years ago

OK, I will prepare the appropriate PR including a testcase tonight or tomorrow.

Am 22/01/15 um 17:29 schrieb Andrew E Slaughter:

I think that this type of numbering would be fine. @dasmy https://github.com/dasmy if you create a pull request I will take a closer look and fix up the error that you are still getting.

Also, could you add a simple test for the desired behavior. For example, utilize simple_diffusion.i but enable |sequence=true| and set |output_on='initial timestep_end nonlinear'|. This should produce a whole series of files. Thanks.

— Reply to this email directly or view it on GitHub https://github.com/idaholab/moose/issues/4573#issuecomment-71049176.

Dr. Mathias Winkel Centre for Computational Medicine in Cardiology (CCMC)

University of Lugano Institute of Computational Science Via Giuseppe Buffi 13 CH-6900 Lugano

Tel: +41 (0)58 666 4973 Email: mathias.winkel@usi.ch Web: http://icsweb.inf.unisi.ch/cms/index.php/people/141-mathias-winkel.html

friedmud commented 9 years ago

Thanks! On Thu, Jan 22, 2015 at 1:47 PM Mathias Winkel notifications@github.com wrote:

OK, I will prepare the appropriate PR including a testcase tonight or tomorrow.

Am 22/01/15 um 17:29 schrieb Andrew E Slaughter:

I think that this type of numbering would be fine. @dasmy https://github.com/dasmy if you create a pull request I will take a closer look and fix up the error that you are still getting.

Also, could you add a simple test for the desired behavior. For example, utilize simple_diffusion.i but enable |sequence=true| and set |output_on='initial timestep_end nonlinear'|. This should produce a whole series of files. Thanks.

— Reply to this email directly or view it on GitHub https://github.com/idaholab/moose/issues/4573#issuecomment-71049176.

Dr. Mathias Winkel Centre for Computational Medicine in Cardiology (CCMC)

University of Lugano Institute of Computational Science Via Giuseppe Buffi 13 CH-6900 Lugano

Tel: +41 (0)58 666 4973 Email: mathias.winkel@usi.ch Web: http://icsweb.inf.unisi.ch/cms/index.php/people/141-mathias-winkel.html

— Reply to this email directly or view it on GitHub https://github.com/idaholab/moose/issues/4573#issuecomment-71075082.