IPP Software Navigation Tools IPP Links Communication Pan-STARRS Links

Opened 22 years ago

Closed 22 years ago

Last modified 22 years ago

#20 closed defect (fixed)

Reduce number of numeric datatypes for psImage/psVector

Reported by: robert.desonia@… Owned by: Paul Price
Priority: high Milestone:
Component: PSLib SDRS Version: unspecified
Severity: major Keywords:
Cc: Eric.VanAlst@…

Description

Currently on the list of data types to support are:

int8 (i.e., char on most systems)
uint8 (i.e., unsigned char on most systems)
int16 (i.e., short int on most systems)
uint16 (i.e., unsigned short int on most systems)
int32 (i.e., int on most systems)
uint32 (i.e., unsigned int on most systems)
long (32 or 64 bit, depending on system, probably should be int64_t)
unsigned long (probably should be uint64_t)
float
complex float
double
complex double

(i.e., nearly every native numeric data type in C)

This is a very long list and really a burden to support. For instance, to add
two images, the code would have to deal with every combination of the list and a
test case would have to be created for each combination.

Is it envisioned that Pan-STARRS will use every one of these types internally?
I recommend that the internal representation be limited to the minimum number of
required types and keep such an exhaustive list for just file I/O. Do we need
to keep all of the integer representations internally, or would it suffice to
cast most of them to/from float/double for file I/O? If it is determined that
the IPP requires either single or double precision floating points, do we really
need to support both?

What would we lose if, for example, we just keep uint8 (mask), float (image) and
complex float (fourier-domain image) for internal data types?

-rdd

Change History (11)

comment:1 by robert.desonia@…, 22 years ago

Severity: normalmajor

Gene,

I think we are not quite on the same page yet. I used addition of two images as
a trivial example, but my concerns are more with supporting 12 datatypes for all
of the psImage/psVector functionality desired. That includes the statistical
functions, histograms, the fitting routines, LUD, matrix operations, etc. For
FFT, for instance, it is defined only valid for floating-point values, but do we
want to do that sort of thing for things everything or spend significant amount
of time building and testing something that will not be needed for PS1?

-rdd

Eugene Magnier wrote:

hi robert,

I am sympathetic to this concern, especially wrt the testing regime.
It is likely that all of the IPP functionality can be satisfied with the
restricted set of three types you have listed. However, we would like
to encourage you to write the functions in such a way that extension to
multiple types is simple or even trivial. The fact is that the
difference between the different data types, in terms of programming, is
generally minimal (eg, change 'psS8' to 'psS32'). Another difference is
the value of an over/underflow, but again these are rigid changes of one
word for another (value of 0xff vs 0xffff, etc). Both of these kinds of
automatic changes can be easily facilitated with #define constructs.

The biggest concern is the BinaryOp function, in which there are many
potential combinations (4 psDimen values x 9 psType values x 9 psType
values x 20 operators = 6480 combinations). In fact, the situation is
not nearly so desperate because you can have the computer generate both
the code and the test data/suite. You really only need to maintain the
code that corresponds to the 4 + 9 + 20 distinct entries.

I'm attaching sample code which shows how this can be done for the
BinaryOp function (not super-rigorous, but the point should be clear).

One compromise point is combinations of types, ie a float image times an
int image. This is a danger because of overflow and choice of output
datatype. I think conversions between types should require more care
than that, so let's make it a requirement than BinaryOp report an error
if in1->type.type != in2->type.type.

aloha
gene

comment:2 by eugene, 22 years ago

(discussion added for clarity to bugzilla from email)

It is certainly true that not all datatypes are relevant or
appropriate for all operations. And I certainly do not want to put
too much of a burden on you and your team at this stage. So, I will
assent to reducing the supported types to a subset of all possible
data types for the time being. However, it is not quite so simple as
saying we will only use: uint8, float, complex float. For instance,
int16 is the native format of all nearly CCD data, and some of the
operations are more accurately performed on that basis (eg, bias
subtraction). Another example is the set of polynomial fitting
functions. We must support doubles to handle the needed astrometric
precission, but we must support floats to avoid excessing data volumes
for images.

I have made a list of the functions (or groups) and the datatypes that
I feel are necessary for the IPP. I definitely insist that the
remaining datatypes continue as part of the data structures (psVector
and psImage); functions which are not defined for specific datatypes
should give an error if it encounters an invalid type.

One point I stronly want to stress: while it may seem useful to code
to the minimal set of data types, it would be unfortunate if the code
were produced without planning for other data types in the future.
Please keep in mind that, in a science analysis system like this, we
do not always know now what we may need in 2 years. If data types are
excessively hardwired, we may find ourselves re-doing alot of work in
the future. So, even if we do not implement the code or tests for the
other data types right now, the code should be produced so that these
additions can be made later on, as I showed in that example with
BinaryOp().

Below, I have the list of required types for the different functions
(note my type abreviations: Sn, Un : signed/unsigned int of n bits;
Fn, Cn : floating pt, complex of n bits). Note that psMath (BinaryOp
and UnaryOp) and the pixel movement functions have a large list. I
expect you will use # define constructs like those I sent yesterday to
simplify this coding.

A couple other notes:

  • I/O of complex types. we will need to specify what it means to write out a complex image. One option to which I am leaning is to use two FITS extensions for the real and imaginary parts.
  • type conversions: we need a function to perform type conversions. psImageCopy has been extended to include a psElemType argument, which if NULL means 'use the original type' but if specified means 'convert to this type'.
  • complex types: we need additional functions to convert to / from real and complex numbers. we currently specified a function to return the real part of a complex image (R = f(C)), but we need an analogous function to return the imaginary part (I = F(C)) and to reverse the process (C = f(R,I)).

comment:3 by eugene, 22 years ago

we have converged on using a minimal subset of datatypes, specified below.
These are also specified in the PSLib SDRS.

psMinimize.h: F32, F64
psFFT.h: F32, C32
psMatrix.h: F32, F64
psMath.h: U8, U16, S8, S16, F32, F64, C32, C64
psSort.h: S8, U16, F32, F64
psStats.h: S8, U16, F32, F64

psImage.h:

pixel movements: U8, U16, S8, S16, F32, F64, C32, C64

psImageAlloc
psImageSubset
psImageFree
psImageFreePixels
psImageFreeChildren
psImageCopy
psImageRotate
psImageRoll
psImageClip
psImageClipNaN
psImageOverlaySection

pixel analysis: S8, U16, F32, F64

psImageSlice
psImageCut
psImageRadialCut
psImageGetStats
psImageHistogram
psImageFitPolynomial
psImageEvalPolynomial

I/O: S8, S16, S32, F32, F64, C32

psImageReadSection
psImageFReadSection
psImageWriteSection
psImageFWriteSection

comment:4 by eugene, 22 years ago

Resolution: fixed
Status: newclosed

comment:5 by Eric.VanAlst@…, 22 years ago

Resolution: fixed
Status: closedreopened

In the list of data types for functions in the bug resolution there are two
specifications that are not currently met in SDR-06:
psVectorStats should be valid for data type psS8 but psU8 is specified in SDR.
psImageReadSection and psImageWriteSection should valid for data types psS8,
psS16, psS32, psF32, psF64, psC32 while the SDR specifies psU8, psU16, psU32,
psF32, psF64.

Should the resolution be changed or the SDR?

comment:6 by eugene, 22 years ago

Owner: changed from eugene to Paul Price
Status: reopenednew

comment:7 by Paul Price, 22 years ago

Resolution: fixed
Status: newclosed

Fixed it in the SDRS.

Note also bug 118; as a result of this, I changed the types for the
psImage{Read,Write}Section to psU8, psS16, psS32, psF32, psF64, as these
correspond to the FITS standard.

comment:8 by Paul Price, 22 years ago

Cc: Eric.VanAlst@… added

comment:9 by Paul Price, 22 years ago

* Bug 28 has been marked as a duplicate of this bug. *

comment:10 by Paul Price, 22 years ago

Keywords: VERIFIED added

Should be fixed in SDRS-07 and ADD-06 (7 September 2004).

comment:11 by Paul Price, 22 years ago

Keywords: VERIFIED removed
Note: See TracTickets for help on using tickets.