[petsc-dev] SETERRQ in fortran

[Smith, Barry F.]

> On Jan 31, 2018, at 7:21 AM, Jed Brown <jed at jedbrown.org> wrote:
> 
> "Smith, Barry F." <bsmith at mcs.anl.gov> writes:
> 
>>> On Jan 5, 2018, at 5:00 PM, Jed Brown <jed at jedbrown.org> wrote:
>>> 
>>> "Smith, Barry F." <bsmith at mcs.anl.gov> writes:
>>> 
>>>>> On Jan 5, 2018, at 4:18 PM, Jed Brown <jed at jedbrown.org> wrote:
>>>>> 
>>>>> "Smith, Barry F." <bsmith at mcs.anl.gov> writes:
>>>>> 
>>>>>>> On Jan 5, 2018, at 12:45 PM, Jed Brown <jed at jedbrown.org> wrote:
>>>>>>> 
>>>>>>> "Smith, Barry F." <bsmith at mcs.anl.gov> writes:
>>>>>>> 
>>>>>>>>> On Jan 4, 2018, at 5:10 PM, Blaise A Bourdin <bourdin at lsu.edu> wrote:
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> On Jan 4, 2018, at 3:16 PM, Smith, Barry F. <bsmith at mcs.anl.gov> wrote:
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> It's changed a bit.  It is better but you need to understand how the new one works, so take a few minutes to see how it works before converting.
>>>>>>>>> Got it.
>>>>>>>>> An example or a link to the fortran macro definition from the man page would be nice 
>>>>>>>>> I am confused about the rationale for putting the endif in the macro, though.
>>>>>>>> 
>>>>>>>> It matches the C paradigm
>>>>>>> 
>>>>>>> Hardly.
>>>>>> 
>>>>>> It matches the paradigm as close as can be reasonable done.
>>>>>> 
>>>>>> I debated putting the then into the macros also.
>>>>>> 
>>>>>>> #define SETERRQ(c,ierr,s)  then ;call PetscError(c,ierr,0,s);return;endif
>>>>>> 
>>>>>> So usage would be 
>>>>>> 
>>>>>>    if (bad) SETERRQ(); 
>>>>>> 
>>>>>> would that be better.
>>>>> 
>>>>> No, Fortran isn't C.
>>>>> 
>>>>> if (bad) then
>>>>>    SETERRQ(...)
>>>>> endif
>>>>> 
>>>>> It doesn't get used so much from Fortran that we need to conceal the
>>>>> language constructs.
>>>> 
>>>>  It will, eventually I want all Fortran examples/tests to have checks on every call (like with have in C).
>>> 
>>> CHKERRQ does the if internally, so it also has the endif.
>> 
>>  What is the relevance of this statement.
> 
> "checks on every call" are relevant to CHKERRQ, not SETERRQ.  CHKERRQ is
> self-contained because it includes the entire if-then-endif.
> 
>>> SETERRA/SETERRQ is used a total of 34 times in 17 Fortran files.
>>> SETERRQ is used a median of zero times and an average of less than 1 in
>>> the C examples.
>> 
>>   I am not sure why you are saying this. My resistance to change has nothing to do with how often it is used. 
> 
> I thought you were concerned with the clutter of
> 
>  if (cond) then SETERRQ(...); endif
> 
> instead of
> 
>  if (cond) then SETERRQ(...)
> 
>>   I am leaning to changing it but don't want to until all the test harness branches etc get into master. So it will be a few days.
> 
> Okay to make this change now?

   Feel free to make the change.

> 
>>> 
>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> This Fortran:
>>>>>>> 
>>>>>>> #define SETERRQ(c,ierr,s)  ;call PetscError(c,ierr,0,s);return;endif
>>>>>>> 
>>>>>>> This would be like writing this C
>>>>>>> 
>>>>>>> #define SETERRQ(c,ierr,s) return PetscError(...); }
>>>>>>> 
>>>>>>> to be used like
>>>>>>> 
>>>>>>> if (BAD) { SETERRQ(comm, ierr, "why")
>>>>>>> 
>>>>>>> which is just bananas and still not as gross as the Fortran.  You might
>>>>>>> not have noticed this because SETERRQ is not called from any of PETSc's
>>>>>>> Fortran examples.
>>>>>> 
>>>>>>  But SETERRA() is and has the same pattern.
>>>>> 
>>>>> It isn't syntactically correct when !defined(PETSC_USE_ERRORCHECKING).
>>>>> The endif isn't going to kill anyone and pulling it out of the macro
>>>>> will make it easier to understand and avoid the circus antics when used
>>>>> in any context other than a positive conditional with no else clause.
>>>> 
>>>>  I'll take this under advisement. Of course in our examples the endif will ALWAYS be on the same line as the rest. Using three lines for a SETERRQ() is ugly.
>>>> 
>>>> 
>>>>> 
>>>>>>> 
>>>>>>>>> Beside not having unmatched if / end if in my code, in a select case construct, I have to write something as ugly as
>>>>>>>>> 
>>>>>>>>> select case (i)
>>>>>>>>> 	case(1) 
>>>>>>>>> 		!do something
>>>>>>>>> 	case(2)
>>>>>>>>> 		!do something else
>>>>>>>>> 	case default
>>>>>>>>> 		if (0 == 0) then
>>>>>>>>> 			SETERRQ(PETSC_COMM_WORLD,PETSC_ERR_ARG_OUTOFRANG,”invalid value”)
>>>>>>>>> end select
>>>>>>>>> 
>>>>>>>> What is ugly about this ? except that you put the SETERRQ on a new line which you did not need to do.
>>>>>>> 
>>>>>>> Reread the above code.  Requiring the dummy opening if statement is nuts.
>>>>>> 
>>>>>> Agreed. He should not use SETERRQ() in this case, should call the error functions directly)
>>>>>> 
>>>>>>> 
>>>>>>>> How do you want to write it so it is prettier?
>>>>>>> 
>>>>>>> SETERRQ should not include that endif.  CHKERRQ has the opening if and
>>>>>>> thus needs the closing too (so it's as intended).  Also note that your
>>>>>>> first reply to Blaise was talking about CHKERRQ when he was asking about
>>>>>>> SETERRQ.
>>>>>> 
>>>>>>  Hmm, I'm not sure about. Oh well, it doesn't matter. You have convinced me of anything.