Hi all,

I attached the test_dmd48.py file to explain, with an example, the issue we encounter with current DM.

Feel free to comment.

PYves

In datamodel.txt the dimensions of the covar HDU are NROW*3:
https://github.com/Subaru-PFS/datamodel/blob/6930c77dc7b26daa6904bce59c5dce5320e32b8f/datamodel.txt#L369
but the assertion below checks for 3*NCOL:
https://github.com/Subaru-PFS/datamodel/blob/6930c77dc7b26daa6904bce59c5dce5320e32b8f/python/pfs/datamodel/pfsSpectrum.py#L275

this should be changed to
assert self.covar.shape == (self.length, 3)

Using a covar array of dimensions (10, 3) [which is legitimate according to datamodel.txt for constructing a PsfObject ]:

covar=np.array([np.ones(10), np.zeros(10), np.zeros(10)])

ct = covar.transpose()

obj = PfsObject(target=target, observations=observations, wavelength=np.array(wavels), flux=np.array(fluxls), mask=np.zeros(10), sky=np.zeros(10), covar=ct, covar2=np.random.rand(2, 2), flags=MaskHelper())

the following error is raised:

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-19-3ab70bf49470> in <module>()
      7                     covar=ct,
      8                     covar2=np.random.rand(2, 2),
----> 9                     flags=MaskHelper())

~/LSST/stack/v16/stack/miniconda3-4.3.21-10a4fa6/DarwinX86/datamodel/5.0-13-g6930c77/python/pfs/datamodel/pfsSpectrum.py in __init__(self, target, observations, wavelength, flux, mask, sky, covar, covar2, flags)
    249         self.covar2 = covar2
    250         self.numExp = len(self.observations)
--> 251         super().__init__(target, wavelength, flux, mask, flags)
    252 
    253     @property

~/LSST/stack/v16/stack/miniconda3-4.3.21-10a4fa6/DarwinX86/datamodel/5.0-13-g6930c77/python/pfs/datamodel/pfsSpectrum.py in __init__(self, target, wavelength, flux, mask, flags)
     39 
     40         self.length = len(wavelength)
---> 41         self.validate()
     42 
     43     def validate(self):

~/LSST/stack/v16/stack/miniconda3-4.3.21-10a4fa6/DarwinX86/datamodel/5.0-13-g6930c77/python/pfs/datamodel/pfsSpectrum.py in validate(self)
    273         assert len(self.observations) == self.numExp
    274         assert self.sky.shape == (self.length,)
--> 275         assert self.covar.shape == (3, self.length)
    276         assert self.covar2.ndim == 2
    277 

AssertionError:

Could you check this?  The datamodel discusses FITS specs which are transposed with respect to C/python and I thought that I'd tested the datamodel code

I checked again, and indeed I was wrong: the code as it stands is correct, and the covar numpy should have a shape of (3, length).

The datamodel.txt specifies the dimensions of the covar HDU array as NROW*3. These are the dimensions for the FITS image, which means that NAXIS1=NROW, and NAXIS2=3. The corresponding numpy array would be transposed, so would have a shape (3, length). Which is what the code as it stands asserts.

This is the first of two points in this issue. The second point, about the non-presence of expId is the subject of DAMD-49.

So I will close this ticket with no further action in a day or so, to allow the creator to comment.

I think the point is not that it isn't correct, it's that "COVAR would be easier to use if given as columns instead of rows" (per Guillaume PERNOT).

Ok, I used BinTable HDU for covar, instead of Image. I was not aware of the difference between BinTable and Image. I'll modify my code in the light of this.

Some extra information in relation to the intended COVAR shape in python of (3, length). This may be useful if we revisit this issue in the future.

https://github.com/Subaru-PFS/datamodel/blob/6930c77dc7b26daa6904bce59c5dce5320e32b8f/datamodel.txt#L298
https://github.com/Subaru-PFS/datamodel/blob/6930c77dc7b26daa6904bce59c5dce5320e32b8f/python/pfs/datamodel/utils.py#L54

Will close the ticket as proposed last week. I will add some additional information in datamodel.txt to emphasize further the shape convention for image data further, but that will be a separate ticket.