[petsc-dev] TS failures in master

[Barry Smith]

> On Apr 29, 2017, at 11:20 AM, Lisandro Dalcin <dalcinl at gmail.com> wrote:
> 
> On 29 April 2017 at 19:12, Satish Balay <balay at mcs.anl.gov> wrote:
>> So the fix is to print the same info for both single and double
>> precision builds.
>> 
>> if (adapt->dt_max < PETSC_MAX_REAL) {ierr = PetscViewerASCIIPrintf(viewer,"  maximum allowed timestep %g\n",(double)adapt->dt_max);CHKERRQ(ierr);}
>> if (adapt->dt_min > 0) {ierr = PetscViewerASCIIPrintf(viewer,"  minimum allowed timestep %g\n",(double)adapt->dt_min);CHKERRQ(ierr);}
>> 
>> Its not clear why you think that info is irrelavent. Even if the
>> values are out of rage (or default values) - its always best to print
>> out the value. Otherwise the user has no clue what the value used.
>> 
> 
> It is irrelevant because if dt_min==0 or dt_max==PETSC_MAX_REAL, it
> means that that the controller is effectively not limiting the time
> step to min/max values.

  Lisandro,

   Even though it may be "irrelevant" it could be/is confusing to users to sometimes have something printed and sometimes not. They may conclude that there is a bug or misunderstand the output due to this by concluding there probably is some bound, it is just not being printed. 

   An alternative to not printing any thing when 0 is to print a different message indicating no clipping, for example something like

>> if (adapt->dt_min > 0) {
>>   ierr = PetscViewerASCIIPrintf(viewer,"  minimum allowed timestep %g\n",(double)adapt->dt_min);CHKERRQ(ierr);
>> } else {
      ierr = PetscViewerASCIIPrintf(viewer,"  no minimum on the allowed timestep\n");CHKERRQ(ierr);
}

   So if you really don't want to have it print the 0 or the MAX please use this construct for clarity. Is that reasonable?

  Barry

Though personally I am fine with printing the zero just as zero since I don't users are confused by that.


> 
>> And removing the conditional avoids the 'different output' for
>> different runs [in this case difference is in precision - not
>> parameters] problem.
> 
> The right fix is:  1) use appropriate default values for dt_min/dt_max
> that make sense for single vs. double precision (either as you did in
> the second hunk of you commit or using let say dt_max=1e20) and 2)
> update the test output.
> 
> 
> -- 
> Lisandro Dalcin
> ============
> Research Scientist
> Computer, Electrical and Mathematical Sciences & Engineering (CEMSE)
> Extreme Computing Research Center (ECRC)
> King Abdullah University of Science and Technology (KAUST)
> http://ecrc.kaust.edu.sa/
> 
> 4700 King Abdullah University of Science and Technology
> al-Khawarizmi Bldg (Bldg 1), Office # 0109
> Thuwal 23955-6900, Kingdom of Saudi Arabia
> http://www.kaust.edu.sa
> 
> Office Phone: +966 12 808-0459