Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format of "skyver" etc. differ for different coadd/*/*ccds* files #379

Closed
geordie666 opened this issue May 3, 2019 · 6 comments
Closed

Format of "skyver" etc. differ for different coadd/*/*ccds* files #379

geordie666 opened this issue May 3, 2019 · 6 comments

Comments

@geordie666
Copy link
Contributor

@geordie666 geordie666 commented May 3, 2019

The format of the character columns in the 90prime-mosaic vs. decam *-ccds.fits files differs:

90prime-mosaic/coadd/132/1320p317/legacysurvey-1320p317-ccds.fits
('skyver', '|S19')
('wcsver', '|S1')
('psfver', '|S19')
('skyplver', '|S4')
('wcsplver', '|S4')
('psfplver', '|S4')
decam/coadd/132/1320p317/legacysurvey-1320p317-ccds.fits
('skyver', '|S8')
('wcsver', '|S1')
('psfver', '|S7')
('skyplver', '|S8')
('wcsplver', '|S5')
('psfplver', '|S8')

I don't know whether we care much, in this instance, given that this is just code versioning information. If anybody decides we don't care, feel free to close this issue without addressing.

@moustakas
Copy link
Contributor

@moustakas moustakas commented May 5, 2019

The skyver, wcsver and psfver are a little funky because we record the git tag plus the most recent commit. So, for example, the 90prime-mosaic file you indicate has, e.g., DR8.0.4-3-g67709eb4 while the decam CCDs file has DR8.0.1 (for some, not all images). So we could fix this column at S19, but that could bite us if we change to tag names that have more strings.

Meanwhile, I'm a little confused by the range of lengths of the [sky,wcs,psf]plver columns, which typically contain things like V2.0 (i.e., the CP PLVER). Specifically, I don't understand why the DECam CCDs file has S8.

I don't think any of these issues are show-stoppers, and since I can't think of an easy fix, I think we should just close this.

Oh, and for the record wcsver is a blank string for both cameras.

@dstndstn
Copy link
Member

@dstndstn dstndstn commented May 14, 2019

wcsver: we used to have separate WCS calib files; now we just do it on the fly.

@moustakas
Copy link
Contributor

@moustakas moustakas commented Dec 23, 2019

Related to #459.

@moustakas
Copy link
Contributor

@moustakas moustakas commented Jan 15, 2020

@geordie666 can you check where we are on this issue in dr9d. Then we should either put in a fix or close this.

@geordie666
Copy link
Contributor Author

@geordie666 geordie666 commented Jan 15, 2020

@moustakas: the issue still persists in dr9d, for example:

import fitsio  
n = fitsio.read("/global/cscratch1/sd/ziyaoz/dr9d-north/coadd/134/1349p300/legacysurvey-1349p300-ccds.fits") 
s = fitsio.read("/global/cscratch1/sd/ziyaoz/dr9d-south/coadd/142/1425p310/legacysurvey-1425p310-ccds.fits") 
print(sorted(set(n.dtype.descr) - set(s.dtype.descr))) 
print(sorted(set(s.dtype.descr) - set(n.dtype.descr)))                                                                                                             
OUTPUT:
[('camera', '|S7'), ('ccdname', '|S4'), ('plver', '|S4'), ('psfplver', '|S4'), ('psfver', '|S18'), ('skyplver', '|S4'), ('wcsplver', '|S4')]
[('camera', '|S5'), ('ccdname', '|S3'), ('plver', '|S7'), ('psfplver', '|S7'), ('psfver', '|S20'), ('skyplver', '|S7'), ('wcsplver', '|S7')]

There are a number of issues like this. As you noted, #459 is another. This is probably to do with building the data types on-the-fly rather than enforcing/asserting a set data model when creating structured arrays.

I don't feel strongly that we have to fix this, but it would be nice to have a set data model across all of the Legacy Survey files.

@moustakas
Copy link
Contributor

@moustakas moustakas commented Mar 9, 2020

I don't believe we're going to fix this; closing.

@moustakas moustakas closed this Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants