Re: [Rd] Unneeded if statements in RealFromComplex C code

2021-09-10 Thread Martin Maechler
> Hervé Pagès 
> on Thu, 9 Sep 2021 17:54:06 -0700 writes:

> Hi,

> I just stumbled across these 2 lines in RealFromComplex (lines 208 & 209 
> in src/main/coerce.c):

> double attribute_hidden
> RealFromComplex(Rcomplex x, int *warn)
> {
>   if (ISNAN(x.r) || ISNAN(x.i))
>   return NA_REAL;
>   if (ISNAN(x.r)) return x.r;<- line 208
>   if (ISNAN(x.i)) return NA_REAL;<- line 209
>   if (x.i != 0)
>  *warn |= WARN_IMAG;
>   return x.r;
> }

> They were added in 2015 (revision 69410).

by me.  "Of course" the intent at the time was to  *replace* the
previous 2 lines and return NA/NaN of the "exact same kind"

but in the mean time, I have learned that trying to preserve
exact *kinds* of NaN / NA is typically not platform portable,
anyway because compiler/library optimizations and
implementations are pretty "free to do what they want" with these.

> They don't serve any purpose and might slow things down a little (unless 
> compiler optimization is able to ignore them). In any case they should 
> probably be removed.

I've cleaned up now, indeed back compatibly, i.e., removing both
lines as you suggested.

Thank you, Hervé!

Martin


> Cheers,
> H.

> -- 
> Hervé Pagès

> Bioconductor Core Team
> hpages.on.git...@gmail.com

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] Unneeded if statements in RealFromComplex C code

2021-09-10 Thread Hervé Pagès

Thanks Martin!

Best,
H.

On 10/09/2021 02:24, Martin Maechler wrote:

Hervé Pagès
 on Thu, 9 Sep 2021 17:54:06 -0700 writes:


 > Hi,

 > I just stumbled across these 2 lines in RealFromComplex (lines 208 & 209
 > in src/main/coerce.c):

 > double attribute_hidden
 > RealFromComplex(Rcomplex x, int *warn)
 > {
 >   if (ISNAN(x.r) || ISNAN(x.i))
 >   return NA_REAL;
 >   if (ISNAN(x.r)) return x.r;<- line 208
 >   if (ISNAN(x.i)) return NA_REAL;<- line 209
 >   if (x.i != 0)
 >  *warn |= WARN_IMAG;
 >   return x.r;
 > }

 > They were added in 2015 (revision 69410).

by me.  "Of course" the intent at the time was to  *replace* the
previous 2 lines and return NA/NaN of the "exact same kind"

but in the mean time, I have learned that trying to preserve
exact *kinds* of NaN / NA is typically not platform portable,
anyway because compiler/library optimizations and
implementations are pretty "free to do what they want" with these.

 > They don't serve any purpose and might slow things down a little (unless
 > compiler optimization is able to ignore them). In any case they should
 > probably be removed.

I've cleaned up now, indeed back compatibly, i.e., removing both
lines as you suggested.

Thank you, Hervé!

Martin


 > Cheers,
 > H.

 > --
 > Hervé Pagès

 > Bioconductor Core Team
 > hpages.on.git...@gmail.com



--
Hervé Pagès

Bioconductor Core Team
hpages.on.git...@gmail.com

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread Duncan Murdoch

On 10/09/2021 11:29 a.m., Hervé Pagès wrote:

Hi,

The first warning below is unexpected and confusing:

> as.raw(c(3e9, 5.1))
[1] 00 05
Warning messages:
1: NAs introduced by coercion to integer range
2: out-of-range values treated as 0 in coercion to raw

The reason we get it is that coercion from numeric to raw is currently
implemented on top of coercion from numeric to int (file
src/main/coerce.c, lines 700-710):

  case REALSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  tmp = IntegerFromReal(REAL_ELT(v, i), );
  if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;

The first warning comes from the call to IntegerFromReal().

The following code avoids the spurious warning and is also simpler and
slightly faster:

  case REALSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  double vi = REAL_ELT(v, i);
  if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;


Doesn't that give different results in case vi is so large that "(int) 
vi" overflows?  (I don't know what the C standard says, but some online 
references say that behaviour is implementation dependent.)


For example, if

  vi = 1.0 +  INT_MAX;

wouldn't "(int) vi" be equal to a small integer?

Duncan Murdoch




Coercion from complex to raw has the same problem:

> as.raw(c(3e9+0i, 5.1))
[1] 00 05
Warning messages:
1: NAs introduced by coercion to integer range
2: out-of-range values treated as 0 in coercion to raw

Current implementation (file src/main/coerce.c, lines 711-721):

  case CPLXSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  tmp = IntegerFromComplex(COMPLEX_ELT(v, i), );
  if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;

This implementation has the following additional problem when the
supplied complex has a nonzero imaginary part:

> as.raw(300+4i)
[1] 00
Warning messages:
1: imaginary parts discarded in coercion
2: out-of-range values treated as 0 in coercion to raw

> as.raw(3e9+4i)
[1] 00
Warning messages:
1: NAs introduced by coercion to integer range
2: out-of-range values treated as 0 in coercion to raw

In one case we get a warning about the discarding of the imaginary part
but not the other case, which is unexpected. We should see the exact
same warning (or warnings) in both cases.

With the following fix we only get the warning about the discarding of
the imaginary part if we are not in a "out-of-range values treated as 0
in coercion to raw" situation:

  case CPLXSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  Rcomplex vi = COMPLEX_ELT(v, i);
  if(ISNAN(vi.r) || ISNAN(vi.i) || (tmp = (int) vi.r) < 0 ||
tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  } else {
  if(vi.i != 0.0)
  warn |= WARN_IMAG;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;

Finally, coercion from character to raw has the same problem and its
code can be fixed in a similar manner:

> as.raw(c("3e9", 5.1))
[1] 00 05
Warning messages:
1: NAs introduced by coercion to integer range
2: out-of-range values treated as 0 in coercion to raw

Cheers,
H.




__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


[Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread Hervé Pagès

Hi,

The first warning below is unexpected and confusing:

  > as.raw(c(3e9, 5.1))
  [1] 00 05
  Warning messages:
  1: NAs introduced by coercion to integer range
  2: out-of-range values treated as 0 in coercion to raw

The reason we get it is that coercion from numeric to raw is currently 
implemented on top of coercion from numeric to int (file 
src/main/coerce.c, lines 700-710):


case REALSXP:
for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
tmp = IntegerFromReal(REAL_ELT(v, i), );
if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
tmp = 0;
warn |= WARN_RAW;
}
pa[i] = (Rbyte) tmp;
}
break;

The first warning comes from the call to IntegerFromReal().

The following code avoids the spurious warning and is also simpler and 
slightly faster:


case REALSXP:
for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
double vi = REAL_ELT(v, i);
if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
tmp = 0;
warn |= WARN_RAW;
}
pa[i] = (Rbyte) tmp;
}
break;

Coercion from complex to raw has the same problem:

  > as.raw(c(3e9+0i, 5.1))
  [1] 00 05
  Warning messages:
  1: NAs introduced by coercion to integer range
  2: out-of-range values treated as 0 in coercion to raw

Current implementation (file src/main/coerce.c, lines 711-721):

case CPLXSXP:
for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
tmp = IntegerFromComplex(COMPLEX_ELT(v, i), );
if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
tmp = 0;
warn |= WARN_RAW;
}
pa[i] = (Rbyte) tmp;
}
break;

This implementation has the following additional problem when the 
supplied complex has a nonzero imaginary part:


  > as.raw(300+4i)
  [1] 00
  Warning messages:
  1: imaginary parts discarded in coercion
  2: out-of-range values treated as 0 in coercion to raw

  > as.raw(3e9+4i)
  [1] 00
  Warning messages:
  1: NAs introduced by coercion to integer range
  2: out-of-range values treated as 0 in coercion to raw

In one case we get a warning about the discarding of the imaginary part 
but not the other case, which is unexpected. We should see the exact 
same warning (or warnings) in both cases.


With the following fix we only get the warning about the discarding of 
the imaginary part if we are not in a "out-of-range values treated as 0 
in coercion to raw" situation:


case CPLXSXP:
for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
Rcomplex vi = COMPLEX_ELT(v, i);
if(ISNAN(vi.r) || ISNAN(vi.i) || (tmp = (int) vi.r) < 0 || 
tmp > 255) {

tmp = 0;
warn |= WARN_RAW;
} else {
if(vi.i != 0.0)
warn |= WARN_IMAG;
}
pa[i] = (Rbyte) tmp;
}
break;

Finally, coercion from character to raw has the same problem and its 
code can be fixed in a similar manner:


  > as.raw(c("3e9", 5.1))
  [1] 00 05
  Warning messages:
  1: NAs introduced by coercion to integer range
  2: out-of-range values treated as 0 in coercion to raw

Cheers,
H.


--
Hervé Pagès

Bioconductor Core Team
hpages.on.git...@gmail.com

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread GILLIBERT, Andre
Hello,


Integer overflow is undefined behavior by the C standard.


For instance, on my computer, with GCC 5.4.0, with the optimization level 2, 
the following program never stops:


include 


int main(void) {
for(int i=1; i != 0; i++) {
if ((i & 0xFFF) == 0) {
printf("%d\n", i);
}
}
}



This is due to a compiler optimization, that assumes that the integer can never 
overflow, and so, can never be equal to zero, and so, the for loop should never 
stops. You should always be very cautious when adding two integers, to avoid 
any overflow. There is no problem with unsigned integers.


Similarly, double-to-integer conversions are only safe if the double is in the 
range [INT_MIN to INT_MAX]


The standard contains:

When a finite value of real floating type is converted to an integer type other 
than _Bool,
the fractional part is discarded (i.e., the value is truncated toward zero). If 
the value of
the integral part cannot be represented by the integer type, the behavior is 
undefined


The easiest solution to avoid a risk when converting, is to check that the 
double (e.g. vi) is in range [0 to 255] BEFORE converting to an integer.


--

Sincerely

Andr� GILLIBERT


De : R-devel  de la part de Duncan Murdoch 

Envoy� : vendredi 10 septembre 2021 18:12:02
� : Herv� Pag�s; r-devel
Objet : Re: [Rd] Spurious warnings in coercion from double/complex/character to 
raw

ATTENTION: Cet e-mail provient d�une adresse mail ext�rieure au CHU de Rouen. 
Ne cliquez pas sur les liens ou n'ouvrez pas les pi�ces jointes � moins de 
conna�tre l'exp�diteur et de savoir que le contenu est s�r. En cas de doute, 
transf�rer le mail � � DSI, S�curit� � pour analyse. Merci de votre vigilance


On 10/09/2021 11:29 a.m., Herv� Pag�s wrote:
> Hi,
>
> The first warning below is unexpected and confusing:
>
> > as.raw(c(3e9, 5.1))
> [1] 00 05
> Warning messages:
> 1: NAs introduced by coercion to integer range
> 2: out-of-range values treated as 0 in coercion to raw
>
> The reason we get it is that coercion from numeric to raw is currently
> implemented on top of coercion from numeric to int (file
> src/main/coerce.c, lines 700-710):
>
>   case REALSXP:
>   for (i = 0; i < n; i++) {
> //  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
>   tmp = IntegerFromReal(REAL_ELT(v, i), );
>   if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
>   tmp = 0;
>   warn |= WARN_RAW;
>   }
>   pa[i] = (Rbyte) tmp;
>   }
>   break;
>
> The first warning comes from the call to IntegerFromReal().
>
> The following code avoids the spurious warning and is also simpler and
> slightly faster:
>
>   case REALSXP:
>   for (i = 0; i < n; i++) {
> //  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
>   double vi = REAL_ELT(v, i);
>   if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
>   tmp = 0;
>   warn |= WARN_RAW;
>   }
>   pa[i] = (Rbyte) tmp;
>   }
>   break;

Doesn't that give different results in case vi is so large that "(int)
vi" overflows?  (I don't know what the C standard says, but some online
references say that behaviour is implementation dependent.)

For example, if

   vi = 1.0 +  INT_MAX;

wouldn't "(int) vi" be equal to a small integer?

Duncan Murdoch


>
> Coercion from complex to raw has the same problem:
>
> > as.raw(c(3e9+0i, 5.1))
> [1] 00 05
> Warning messages:
> 1: NAs introduced by coercion to integer range
> 2: out-of-range values treated as 0 in coercion to raw
>
> Current implementation (file src/main/coerce.c, lines 711-721):
>
>   case CPLXSXP:
>   for (i = 0; i < n; i++) {
> //  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
>   tmp = IntegerFromComplex(COMPLEX_ELT(v, i), );
>   if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
>   tmp = 0;
>   warn |= WARN_RAW;
>   }
>   pa[i] = (Rbyte) tmp;
>   }
>   break;
>
> This implementation has the following additional problem when the
> supplied complex has a nonzero imaginary part:
>
> > as.raw(300+4i)
> [1] 00
> Warning messages:
> 1: imaginary parts discarded in coercion
> 2: out-of-range values treated as 0 in coercion to raw
>
> > as.raw(3e9+4i)
> [1] 00
> Warning messages:
> 1: NAs introduced by coercion to integer range
> 2: out-of-range values treated as 0 in coercion to raw
>
> In one case we get a warning about the discarding of the imaginary part
> but not the other case, which is unexpected. We should see the exact
> same warning (or warnings) in both cases.
>
> With the following fix we only get the warning about the discarding of
> 

Re: [Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread Hervé Pagès




On 10/09/2021 09:12, Duncan Murdoch wrote:

On 10/09/2021 11:29 a.m., Hervé Pagès wrote:

Hi,

The first warning below is unexpected and confusing:

    > as.raw(c(3e9, 5.1))
    [1] 00 05
    Warning messages:
    1: NAs introduced by coercion to integer range
    2: out-of-range values treated as 0 in coercion to raw

The reason we get it is that coercion from numeric to raw is currently
implemented on top of coercion from numeric to int (file
src/main/coerce.c, lines 700-710):

  case REALSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  tmp = IntegerFromReal(REAL_ELT(v, i), );
  if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;

The first warning comes from the call to IntegerFromReal().

The following code avoids the spurious warning and is also simpler and
slightly faster:

  case REALSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  double vi = REAL_ELT(v, i);
  if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;


Doesn't that give different results in case vi is so large that "(int) 
vi" overflows?  (I don't know what the C standard says, but some online 
references say that behaviour is implementation dependent.)


For example, if

   vi = 1.0 +  INT_MAX;

wouldn't "(int) vi" be equal to a small integer?


Good catch, thanks!

Replacing

if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
tmp = 0;

warn |= WARN_RAW;

}
pa[i] = (Rbyte) tmp;

with

if(ISNAN(vi) || vi <= -1.0 || vi >= 256.0)
 {
tmp = 0;

warn |= WARN_RAW;

} else {
tmp = (int) vi;
}
pa[i] = (Rbyte) tmp;

should address that.

FWIW IntegerFromReal() has a similar risk of int overflow 
(src/main/coerce.c, lines 128-138):


  int attribute_hidden

  IntegerFromReal(double x, int *warn)

  {

  if (ISNAN(x))

  return NA_INTEGER;

  else if (x >= INT_MAX+1. || x <= INT_MIN ) {

  *warn |= WARN_INT_NA;

  return NA_INTEGER;

  }

  return (int) x;

  }



The cast to int will also be an int overflow situation if x is > INT_MAX 
and < INT_MAX+1 so the risk is small! There are other instances of this 
situation in IntegerFromComplex() and IntegerFromString().


More below...



Duncan Murdoch




Coercion from complex to raw has the same problem:

    > as.raw(c(3e9+0i, 5.1))
    [1] 00 05
    Warning messages:
    1: NAs introduced by coercion to integer range
    2: out-of-range values treated as 0 in coercion to raw

Current implementation (file src/main/coerce.c, lines 711-721):

  case CPLXSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  tmp = IntegerFromComplex(COMPLEX_ELT(v, i), );
  if(tmp == NA_INTEGER || tmp < 0 || tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;

This implementation has the following additional problem when the
supplied complex has a nonzero imaginary part:

    > as.raw(300+4i)
    [1] 00
    Warning messages:
    1: imaginary parts discarded in coercion
    2: out-of-range values treated as 0 in coercion to raw

    > as.raw(3e9+4i)
    [1] 00
    Warning messages:
    1: NAs introduced by coercion to integer range
    2: out-of-range values treated as 0 in coercion to raw

In one case we get a warning about the discarding of the imaginary part
but not the other case, which is unexpected. We should see the exact
same warning (or warnings) in both cases.

With the following fix we only get the warning about the discarding of
the imaginary part if we are not in a "out-of-range values treated as 0
in coercion to raw" situation:

  case CPLXSXP:
  for (i = 0; i < n; i++) {
//  if ((i+1) % NINTERRUPT == 0) R_CheckUserInterrupt();
  Rcomplex vi = COMPLEX_ELT(v, i);
  if(ISNAN(vi.r) || ISNAN(vi.i) || (tmp = (int) vi.r) < 0 ||
tmp > 255) {
  tmp = 0;
  warn |= WARN_RAW;
  } else {
  if(vi.i != 0.0)
  warn |= WARN_IMAG;
  }
  pa[i] = (Rbyte) tmp;
  }
  break;


Corrected version:

if(ISNAN(vi.r) || ISNAN(vi.i) || vi.r <= -1.00 ||
 vi.r >= 256.00) {

tmp = 0;

warn |= WARN_RAW;

} else {

tmp = (int) vi.r;
if(vi.i != 0.0)

warn |= WARN_IMAG;

}

pa[i] = (Rbyte) tmp;

Hopefully this time I got it right.

Best,
H.



Finally, coercion from character to 

Re: [Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread Hervé Pagès




On 10/09/2021 12:53, brodie gaslam wrote:



On Friday, September 10, 2021, 03:13:54 PM EDT, Hervé Pagès 
 wrote:

Good catch, thanks!

Replacing

  if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
  tmp = 0;

  warn |= WARN_RAW;

  }
  pa[i] = (Rbyte) tmp;

with

  if(ISNAN(vi) || vi <= -1.0 || vi >= 256.0)
    {
  tmp = 0;

  warn |= WARN_RAW;

  } else {
  tmp = (int) vi;
  }
  pa[i] = (Rbyte) tmp;

should address that.

FWIW IntegerFromReal() has a similar risk of int overflow
(src/main/coerce.c, lines 128-138):


    int attribute_hidden

    IntegerFromReal(double x, int *warn)

    {

    if (ISNAN(x))

    return NA_INTEGER;

    else if (x >= INT_MAX+1. || x <= INT_MIN ) {

    *warn |= WARN_INT_NA;

    return NA_INTEGER;

    }

    return (int) x;

    }



The cast to int will also be an int overflow situation if x is > INT_MAX
and < INT_MAX+1 so the risk is small!


I might be being dense, but it feels this isn't a problem?  Quoting C99
6.3.1.4 again (emph added):


When a finite value of real floating type is converted to an integer
type other than _Bool, **the fractional part is discarded** (i.e., the
value is truncated toward zero). If the value of the integral part
cannot be represented by the integer type, the behavior is undefined.50)


Does the "fractional part is discarded" not save us here?


I think it does. Thanks for clarifying and sorry for the false positive!

H.



Best,

B.




--
Hervé Pagès

Bioconductor Core Team
hpages.on.git...@gmail.com

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] Spurious warnings in coercion from double/complex/character to raw

2021-09-10 Thread brodie gaslam via R-devel


> On Friday, September 10, 2021, 03:13:54 PM EDT, Hervé Pagès 
>  wrote:
>
> Good catch, thanks!
>
> Replacing
>
> if(ISNAN(vi) || (tmp = (int) vi) < 0 || tmp > 255) {
> tmp = 0;
>
> warn |= WARN_RAW;
>
> }
> pa[i] = (Rbyte) tmp;
>
> with
>
> if(ISNAN(vi) || vi <= -1.0 || vi >= 256.0)
>   {
> tmp = 0;
>
> warn |= WARN_RAW;
>
> } else {
> tmp = (int) vi;
> }
> pa[i] = (Rbyte) tmp;
>
> should address that.
>
> FWIW IntegerFromReal() has a similar risk of int overflow
> (src/main/coerce.c, lines 128-138):
>
>
>   int attribute_hidden
>
>   IntegerFromReal(double x, int *warn)
>
>   {
>
>   if (ISNAN(x))
>
>   return NA_INTEGER;
>
>   else if (x >= INT_MAX+1. || x <= INT_MIN ) {
>
>   *warn |= WARN_INT_NA;
>
>   return NA_INTEGER;
>
>   }
>
>   return (int) x;
>
>   }
>
>
>
> The cast to int will also be an int overflow situation if x is > INT_MAX
> and < INT_MAX+1 so the risk is small!

I might be being dense, but it feels this isn't a problem?  Quoting C99
6.3.1.4 again (emph added):

> When a finite value of real floating type is converted to an integer
> type other than _Bool, **the fractional part is discarded** (i.e., the
> value is truncated toward zero). If the value of the integral part
> cannot be represented by the integer type, the behavior is undefined.50)

Does the "fractional part is discarded" not save us here?

Best,

B.

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Bioc-devel] Git lfs support

2021-09-10 Thread Anatoly Sorokin
Hi Hervé,

I've made a new package with metadata.csv, as described in
CreateAnAnnotationPackage
vignette.

The data is located on the public service as a zipped SQLite database. So
I've set both SourceType and DispatchClass as 'Zip'.
>From AnnotationHub::DispatchClassList() I expect that I should have an
unzipped file path. My first question is: how do I get that path?

The second question is: what should I put into make-data.R? The database is
already created, cleaned and validated. All I need for the package is to
download it, unzip and connect.

Thank you,
Anatoly

On Thu, Aug 26, 2021 at 12:59 PM Hervé Pagès 
wrote:

> Hi Anatoly,
>
> Let's keep this conversation on the bioc-devel mailing list where it
> started.
>
> On 26/08/2021 02:46, Anatoly Sorokin wrote:
> > Hi Hervé,
> >
> > thank you for your answer. Does this mean that the SQLite file will be
> > downloaded separately and kept in some cache?
>
> Yes.
>
> >
> > And another question: does this mean that there should be two packages,
> > one for code and another one for the database?
>
> Yes. One is what we call the software package and the other one the
> accompanying data package.
>
> Cheers,
> H.
>
> >
> > On Wed, Aug 25, 2021 at 6:58 AM Hervé Pagès  > > wrote:
> >
> > Hi Anatoly,
> >
> > What kind of data is in your SQLite database? The Bioconductor
> approach
> > for this is to provide the data as a separate data annotation or data
> > experiment package. More precisely, the data itself should go on
> > AnnotationHub or ExperimentHub. It should be associated with a "Hub
> > package", that is, a data package that documents it and explains how
> to
> > retrieve it from AnnotationHub or ExperimentHub. See vignettes in the
> > HubPub package for more information:
> >
> > http://bioconductor.org/packages/HubPub
> > 
> >
> > Best,
> >
> > H.
> >
> > On 24/08/2021 17:53, Nitesh Turaga wrote:
> >  > Hi,
> >  >
> >  > We don’t have LFS support on the Bioconductor git server.
> >  >
> >  > I would suggest finding an alternative.
> >  >
> >  > Best,
> >  >
> >  >
> >  >
> >  >
> >  > Nitesh Turaga
> >  > Scientist II, Department of Data Science,
> >  > Bioconductor Core Team Member
> >  > Dana Farber Cancer Institute
> >  >
> >  >> On Aug 24, 2021, at 6:18 PM, Anatoly Sorokin  > > wrote:
> >  >>
> >  >> Hi all,
> >  >>
> >  >> we have developed a package that provides access to the data in
> > the locally
> >  >> stored SQLite database. The GitHub Action successfully built the
> > package,
> >  >> but in Bioconductor, it got an error. The only reason for the
> > error is that
> >  >> the database itself is stored on GitHub via git-lfs, and the R
> > code got an
> >  >> lfs placeholder instead of the actual database.
> >  >>
> >  >> Is it possible to activate lfs within Bioconductor automatic
> > building
> >  >> server? The database is tiny (80 Mb), but I don't want to have
> > it as a
> >  >> regular git object.
> >  >>
> >  >> Thank you,
> >  >> Anatoly
> >  >>
> >  >>  [[alternative HTML version deleted]]
> >  >>
> >  >> ___
> >  >> Bioc-devel@r-project.org 
> > mailing list
> >  >> https://stat.ethz.ch/mailman/listinfo/bioc-devel
> > 
> >  >
> >  > ___
> >  > Bioc-devel@r-project.org 
> > mailing list
> >  > https://stat.ethz.ch/mailman/listinfo/bioc-devel
> > 
> >  >
> >
> > --
> > Hervé Pagès
> >
> > Bioconductor Core Team
> > hpages.on.git...@gmail.com 
> >
>
> --
> Hervé Pagès
>
> Bioconductor Core Team
> hpages.on.git...@gmail.com
>

[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel


Re: [Bioc-devel] pdfTeX error, Windows build

2021-09-10 Thread Kern, Lori
There does not appear to be an error in the build report in the link you sent 
from yesterday's build. It appears that the error may have been intermittent.


Lori Shepherd

Bioconductor Core Team

Roswell Park Comprehensive Cancer Center

Department of Biostatistics & Bioinformatics

Elm & Carlton Streets

Buffalo, New York 14263


From: Bioc-devel  on behalf of Pedro Madrigal 

Sent: Friday, September 3, 2021 2:57 PM
To: bioc-devel@r-project.org 
Subject: [Bioc-devel] pdfTeX error, Windows build

Hi Bioc-Devel

Do you have any idea of what might be happening here

http://bioconductor.org/checkResults/devel/bioc-LATEST/fCCAC/ 



Best,
Pedro
(Package author and maintainer)
[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list
https://secure-web.cisco.com/1J5STcKxnYfnVxjP_G4R-bGhoFNtUbzD0iESDi5RYUHHHTNHfz4Se8cN9QsMGvd2fX4AM1BXOG7YEUY18fJLgkMLeoM8g4lNvzxPNZcPU01rZt-rQfIDjhhVCF1HH9rV-B8x9EgYdYt2Mnb1lp_QGzhfaR3gpOCOnciUumlQ2vkxQ_uKYtIOR0S4-RIbgvb2WxWASG5DIMak8iRN8hO4s1PybydWxP2fmsZ7ZxSjM9XUH7pz5fSvnSoFW0RiZJ3gDLFjxtASnfU_mfXBQPnU_yCqxRn66boLnFwnHo7lmVhjtpv_JkG69fl-MvAfsjvOT/https%3A%2F%2Fstat.ethz.ch%2Fmailman%2Flistinfo%2Fbioc-devel



This email message may contain legally privileged and/or confidential 
information.  If you are not the intended recipient(s), or the employee or 
agent responsible for the delivery of this message to the intended 
recipient(s), you are hereby notified that any disclosure, copying, 
distribution, or use of this email message is prohibited.  If you have received 
this message in error, please notify the sender immediately by e-mail and 
delete this email message from your computer. Thank you.
[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel


[Bioc-devel] pdfTeX error, Windows build

2021-09-10 Thread Pedro Madrigal
Hi Bioc-Devel

Do you have any idea of what might be happening here

http://bioconductor.org/checkResults/devel/bioc-LATEST/fCCAC/ 



Best,
Pedro
(Package author and maintainer)
[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel


Re: [Bioc-devel] Git lfs support

2021-09-10 Thread Kern, Lori
The paths will be given through the AnnotationHub interface.

I would like to also point out that there is a SQLite  DisplatchClass that 
might be more appropriate? It would load the database automatically using 
AnnotaionDbi::loadDb
For a Zip DispatchClass, It will get the zipped file, unzip, and provide a list 
of the file paths.

make-data.R should be how you created the sqlite database.  It doesn't need to 
be run again. It could be code, sudo-code, or text but should describte how you 
created the data and any relevant source information for the data.

Hope this helps

Cheers,



Lori Shepherd

Bioconductor Core Team

Roswell Park Comprehensive Cancer Center

Department of Biostatistics & Bioinformatics

Elm & Carlton Streets

Buffalo, New York 14263


From: Bioc-devel  on behalf of Anatoly 
Sorokin 
Sent: Friday, September 10, 2021 5:56 AM
To: bioc-devel@r-project.org 
Subject: Re: [Bioc-devel] Git lfs support

Hi Herv�,

I've made a new package with metadata.csv, as described in
CreateAnAnnotationPackage
vignette.

The data is located on the public service as a zipped SQLite database. So
I've set both SourceType and DispatchClass as 'Zip'.
From AnnotationHub::DispatchClassList() I expect that I should have an
unzipped file path. My first question is: how do I get that path?

The second question is: what should I put into make-data.R? The database is
already created, cleaned and validated. All I need for the package is to
download it, unzip and connect.

Thank you,
Anatoly

On Thu, Aug 26, 2021 at 12:59 PM Herv� Pag�s 
wrote:

> Hi Anatoly,
>
> Let's keep this conversation on the bioc-devel mailing list where it
> started.
>
> On 26/08/2021 02:46, Anatoly Sorokin wrote:
> > Hi Herv�,
> >
> > thank you for your answer. Does this mean that the SQLite file will be
> > downloaded separately and kept in some cache?
>
> Yes.
>
> >
> > And another question: does this mean that there should be two packages,
> > one for code and another one for the database?
>
> Yes. One is what we call the software package and the other one the
> accompanying data package.
>
> Cheers,
> H.
>
> >
> > On Wed, Aug 25, 2021 at 6:58 AM Herv� Pag�s  > > wrote:
> >
> > Hi Anatoly,
> >
> > What kind of data is in your SQLite database? The Bioconductor
> approach
> > for this is to provide the data as a separate data annotation or data
> > experiment package. More precisely, the data itself should go on
> > AnnotationHub or ExperimentHub. It should be associated with a "Hub
> > package", that is, a data package that documents it and explains how
> to
> > retrieve it from AnnotationHub or ExperimentHub. See vignettes in the
> > HubPub package for more information:
> >
> > http://bioconductor.org/packages/HubPub
> > 
> >
> > Best,
> >
> > H.
> >
> > On 24/08/2021 17:53, Nitesh Turaga wrote:
> >  > Hi,
> >  >
> >  > We don�t have LFS support on the Bioconductor git server.
> >  >
> >  > I would suggest finding an alternative.
> >  >
> >  > Best,
> >  >
> >  >
> >  >
> >  >
> >  > Nitesh Turaga
> >  > Scientist II, Department of Data Science,
> >  > Bioconductor Core Team Member
> >  > Dana Farber Cancer Institute
> >  >
> >  >> On Aug 24, 2021, at 6:18 PM, Anatoly Sorokin  > > wrote:
> >  >>
> >  >> Hi all,
> >  >>
> >  >> we have developed a package that provides access to the data in
> > the locally
> >  >> stored SQLite database. The GitHub Action successfully built the
> > package,
> >  >> but in Bioconductor, it got an error. The only reason for the
> > error is that
> >  >> the database itself is stored on GitHub via git-lfs, and the R
> > code got an
> >  >> lfs placeholder instead of the actual database.
> >  >>
> >  >> Is it possible to activate lfs within Bioconductor automatic
> > building
> >  >> server? The database is tiny (80 Mb), but I don't want to have
> > it as a
> >  >> regular git object.
> >  >>
> >  >> Thank you,
> >  >> Anatoly
> >  >>
> >  >>  [[alternative HTML version deleted]]
> >  >>
> >  >> ___
> >  >> Bioc-devel@r-project.org 
> > mailing list
> >  >> 
> > https://secure-web.cisco.com/1WUW7ZRdbtpRvJ5xI0GsYHli6Fa14AGQb8WMb4rQ3UJJMaH1UjGkvTOPPE-nuN6rDlQq_KGnu0uVjf2v5v5bsuiqhHF7AUrVpF6bNQkRgCyLo8e-HiA3KLjbJ8_B4XYC58Duri0Sh4_YTCpgpvDPcP6WfBHvGC9oB-4QifRz-gUYg4C65EXHrA7TjMAzj4CHo9a6zIQN4VjeWyW_p5L1ueEaw9Jn8xa9gurW85DezyosVZrZYUNthLQcPeuePvCpK2Or4CP1tuaLXSiLtAzynJYcvyELR4tPWNyvN_qqEpYQC-ld9a0P8zTZAXtTtwbR3/https%3A%2F%2Fstat.ethz.ch%2Fmailman%2Flistinfo%2Fbioc-devel
> > 
> >