Bug in PJSR (or PJSR doc, impact BatchPreprocessing

[bitli]

In BPP 1.34 PI 1087, file BatchPreprocessing-engine.js, line 1384:

Code: [Select]

   if ( a_exp.length > 0 )   {
      exptime = (a_exp.length > 1) ? Math.max( a_exp ) : a_exp[0];
   }
For some reason the Math.max(a_exp) returns NaN. I tested with one file with exposure time 420 and two with 421. The array is correctly reduced to a length of 2, but the test returns Nan.  According to the doc the method Math.max should work with a list of number or an array. However the following sample scripts:

Code: [Select]

Console.writeln(Math.max(3,4,2));
var a = new Array(3,4,2);
Console.writeln(Math.max(a));shows '4' then 'NaN'.

Note sure if the doc is wrong or if this is a problem with PJSR.
A workaround on BPP is to replace the lines by:

Code: [Select]

   if ( a_exp.length > 0 )   {
      exptime = a_exp[0];
      for (var ia_exp = 1; ia_exp<a_exp.length; ia_exp++) {
         if (exptime < a_exp[ia_exp]) {
            exptime = a_exp[ia_exp];
         }
      }
   }(it could be prettier...)

This problem impacts people integrating files with different exposures, as BPP will not take the proper dark but always the one with the longest exposure.  This again may be hidden if you use 'optimize' (which is not always possible).
-- bitli

[Juan Conejero]

Bug confirmed. Both Math.min() and Math.max() are broken in PI 1.8.1 when the argument of these methods is an array.

This is a regression caused by the way we implement the standard Math JavaScript object in PixInsight since version 1.8.1. In previous versions, the standard Math object was completely removed and replaced with a custom, non-ECMA compliant but much more powerful and efficient Math object. Since 1.8.1 and the integration of SpiderMonkey 24, we no longer replace the Math object because JIT-inlined Math methods are faster than our implementations, which cannot be inlined by the JIT compiler. We now preserve standard Math methods (such as Math.min() and Math.max() for example) and extend the Math object with our routines that are not part of ECMA (for example, Math.complexTimeToJD() or Math.stableSum()). Unfortunately, our custom Math.min() and Math.max() accepted nonstandard array arguments, which was a design error that we are paying now.

The fix is two new custom methods, namely:

Number Math.minElem( Array numbers )
Number Math.minElem( Number x1[, Number x2[, ..., Number xN]] )

Number Math.maxElem( Array numbers )
Number Math.maxElem( Number x1[, Number x2[, ..., Number xN]] )

that extend the Math object without causing conflicts. These methods will be available in the next version of the PixInsight Core application, and a new version of BatchPreprocessing will invoke Math.maxElem() in StackEngine.doCalibrate(). Math.min() and Math.max() will be documented correctly:

Number Math.min( Number x1[, Number x2[, ..., Number xN]] )
Number Math.max( Number x1[, Number x2[, ..., Number xN]] )

that is, according to their standard ECMA definitions.

Since this bug does not seem to be causing too many practical problems and the new version is due in a week or so, I prefer to wait instead of releasing an update. Thanks for the bug hunt.