[petsc-dev] [petsc-maint #74896] Re: What was the 64-bit problem with pow()?

[Barry Smith]

On May 31, 2011, at 9:56 PM, Matthew Knepley wrote:

> On Tue, May 31, 2011 at 9:45 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> 
>> 
>> Man that PowInt is one ugly mo-fo.   Since an int is an int is an int why
>> do you need the PowInt? Why not just caste with (int)? Instead of increasing
>> the complexity of PETSc with PowInt?  int is acceptable in PETSc when passed
>> to appropriate system calls that do take an int.  The cases PetscMPIInt and
>> PetscBLASInt are marked with special names to make the code clear but I
>> don't think PowInt is needed.
> 
> 
> I did not want to downcast because PetscPow() works for (Scalar, Int) and
> (Scalar, Real) so automatically casting to (int) is not right.

   But that is exactly what the code does, isn't it? 

#if defined(PETSC_USE_64BIT_INDICES)
typedef int PowInt;
#else
typedef PetscInt PowInt;
#endif

static PetscScalar CPowF(PetscScalar c,PetscInt p)
{
  return PetscPowScalar(c,(PowInt)p)/Factorial(p);
}

Isn't it casting to PowInt which is int?


   Barry



> 
>   Matt
> 
> 
>> 
>> 
>>  Barry
>> 
>> 
>> 
>> On May 30, 2011, at 9:53 AM, Jed Brown wrote:
>> 
>>> On Mon, May 30, 2011 at 16:38, Matthew Knepley <knepley at gmail.com>
>> wrote:
>>> You wrote the code that takes PetscInt p!!! If you do not want that,
>> change it to PetscReal.
>>> 
>>> Right, I was relying on standard promotion rules and forgot about
>> std::pow(scalar, int). The exponent is a loop counter, so it can't
>> reasonably be made a scalar. This whole thing could be implemented without
>> calling pow() in any form, just multiplication, but it's not in a
>> performance-sensitive place and I thought the code would be easier to read
>> this way.
>>> 
>>> The reason I brought this up is that I thought it was worth standardizing
>> a way to get an integer power (this is a common thing to need in adaptive
>> controllers). Should we add a function like PetscPowRealInt()? This seems
>> ugly, but it's also ugly and confusing to deal with these functions
>> resolving to quite different things on C versus C++ builds (with the usual
>> variability in C++ compilers). This code is over a year old so it shouldn't
>> have new portability issues. The existence of a new ticket seems like a
>> defect in the process, but I'm not sure what the best fix is.
>> 
>> 
> 
> 
> -- 
> What most experimenters take for granted before they begin their experiments
> is infinitely more interesting than any results to which their experiments
> lead.
> -- Norbert Wiener
> 
> On Tue, May 31, 2011 at 9:45 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
> 
>  Man that PowInt is one ugly mo-fo.   Since an int is an int is an int why do you need the PowInt? Why not just caste with (int)? Instead of increasing the complexity of PETSc with PowInt?  int is acceptable in PETSc when passed to appropriate system calls that do take an int.  The cases PetscMPIInt and PetscBLASInt are marked with special names to make the code clear but I don't think PowInt is needed.
> 
> I did not want to downcast because PetscPow() works for (Scalar, Int) and (Scalar, Real) so automatically casting to (int) is not right.
> 
>    Matt
>  
> 
> 
>   Barry
> 
> 
> 
> On May 30, 2011, at 9:53 AM, Jed Brown wrote:
> 
> > On Mon, May 30, 2011 at 16:38, Matthew Knepley <knepley at gmail.com> wrote:
> > You wrote the code that takes PetscInt p!!! If you do not want that, change it to PetscReal.
> >
> > Right, I was relying on standard promotion rules and forgot about std::pow(scalar, int). The exponent is a loop counter, so it can't reasonably be made a scalar. This whole thing could be implemented without calling pow() in any form, just multiplication, but it's not in a performance-sensitive place and I thought the code would be easier to read this way.
> >
> > The reason I brought this up is that I thought it was worth standardizing a way to get an integer power (this is a common thing to need in adaptive controllers). Should we add a function like PetscPowRealInt()? This seems ugly, but it's also ugly and confusing to deal with these functions resolving to quite different things on C versus C++ builds (with the usual variability in C++ compilers). This code is over a year old so it shouldn't have new portability issues. The existence of a new ticket seems like a defect in the process, but I'm not sure what the best fix is.
> 
> 
> 
> 
> -- 
> What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.
> -- Norbert Wiener