idaholab / moose

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

Better Error Message for old values / Steady State calcluations #380

Closed permcody closed 10 years ago

permcody commented 10 years ago

Right now one receives a seg fault for using _old or _older values when using the Steady Executioner. In debug mode the error is still vague reporting an out of bounds assertion in MooseArray. A better error message should be provided for this incorrect usage case

andrsd commented 10 years ago

Just a comment on this bug, since it is not trivial to fix: This error is triggered by using operator[on _u_old, _u_older (and maybe others). For those variables, this would require a special array that would assert on using operator(]) in case of steady-state problem and would work just fine on transient ones. All that would make the code slightly less legible (different types for _u and _u_old/older etc.)

I do not see another way how to check this ;-(

permcody commented 10 years ago

I believe this could be fixed by putting in the right callbacks in "Coupled" to track when old or older values have been requested (not just used). Then we could simply see which executioner has been instantiated and throw an error.

YaqiWang commented 10 years ago

I noticed a Cody's resent pull request #2718. I suggest that we remove transient check in MooseVariable completely. Values will be evaluated solely upon request from kernels, auxkernels, materials, userobjects, etc. This will increase the number of bool checks but should not affect CPU time. Users might request wrong coupleable, for example, coupledValueOld instead of coupledValue for a Steady executioner. But this can be detected and warned in Coupleable. In this way, the power method eigenvalue executioner does not have to set the problem to be transient to use the old solutions. FEProblem::isTransient() will simple means if the problem is transient or not.

friedmud commented 10 years ago

Can't do that - users don't request old values - they are simply there. It's a design decision we made a long time ago and we're not changing right now (maybe sometime in the future).

If you need access to old values of variables then as far as MOOSE is concerned you're doing an Transient calculation...

YaqiWang commented 10 years ago

The old, older solution vectors are always there. IMO, this can be changed during executioner construction. At that time, we will know if we are going to need those vectors. A power method executioner needs the old values although it is not a transient calculation. In general, old solution does not mean a solution of the previous time step, but could be a solution of the previous iteration.

friedmud commented 10 years ago

MOOSE doesn't view "previous iteration" as being different from "previous time step". Multiple steps = Transient... period.

YaqiWang commented 10 years ago

Actually there is already a complexity on using PowerMethodExecutioner and DepletionExecutioner. DepletionExecutioner is a real transient executioner. But at every time step, it solves an eigenvalue problem which will use old and older solution vectors. Currently I let PowerMethodExecutioner to save the old and older solution vectors at the beginning and restore them at the end.

YaqiWang commented 10 years ago

Then how to fix issue #2452?

friedmud commented 10 years ago

I don't understand that ticket - nor do I understand why you think you should be able to use "old" and "older" values without the thing being "Transient"

Come by - let's chat about this stuff..... again ;-)