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

tile_id wrong in DRP headers before MJD 60235 #120

Closed
kreckel opened this issue Jul 15, 2024 · 7 comments
Closed

tile_id wrong in DRP headers before MJD 60235 #120

kreckel opened this issue Jul 15, 2024 · 7 comments

Comments

@kreckel
Copy link

kreckel commented Jul 15, 2024

In looking at Dmitry's list related to the standard star acquisition for dithered observations, I noticed that the TILE_ID in the DRP files is incorrect for survey tiles observed before MJD 60256. This was the point (from 60235-60251) when the cryostats warmed up and we completed the ORR and shifted to survey operations, and my spot checking of tiles observed before this point all had the wrong TILD_ID in the DRP output files.

For example, on MJD 60232 a number of survey tiles in the LMC are observed:
https://lvm-viki.lco.cl/tileDetail.html?tile_id=1000238

in the DRP produced files (https://data.sdss5.org/sas/sdsswork/lvm/spectro/redux/1.0.3/1027XX/1027985/60232/) the TILE_ID is wrong (e.g. for lvmSFrame-00006694.fits):
TILE_ID = 1027985

In the raw data frames (https://data.sdss5.org/sas/sdsswork/data/lvm/lco/60232/) the TILE_ID is correct (e.g. for sdR-s-b1-00006694.fits):
TILE_ID = 1000238

The other header information appears correct (ra/dec)

@ajmejia
Copy link
Contributor

ajmejia commented Aug 20, 2024

Hi @kreckel, I looking at the header fix for MJD=60232, expnum=6694 here:

https://github.com/sdss/lvmcore/blob/master/hdrfix/60232/lvmHdrFix-60232.yaml

And is clear where this wrong value is coming from. I'm not sure about the other MJDs (60235 - 60251) because those have no header fix files. I'll look into it in the pipeline, but I don't thing the TILE_ID is changing somewhere in the pipeline.

@havok2063
Copy link
Collaborator

I think @johndonor3 added this hdrFix file. He can probably comment on at least this particular TILE_ID. I think there was a reason we made these updates, perhaps where the original tile ids were incorrect?

@johndonor3
Copy link

Those tile ids correspond to the same location on sky, with the same PA. The only difference is which version of the tiling they are associated with. I did a large hdrFix when we updated the tiling to match any observation of an old tile to the correct new tile (to within 1" or some acceptable margin, I don't recall exactly but could check).

So this was deliberate. Is there a reason we don't want to the newer tile associated with the observation in the pipeline? It's entirely a book keeping decision; the tiles are exactly the same apart from tile_id. And note there are at least 100 hdrFix's doing updates/mappings like this.

@havok2063
Copy link
Collaborator

This is the original commit from lvmcore, sdss/lvmcore@6bb5308

@ajmejia
Copy link
Contributor

ajmejia commented Aug 22, 2024

OK, I see. So it's not a wrong value it's just an update.

The pipeline is for the most part agnostic to tile_id. It only uses this field to resolve the path to the output products. So as long as we are consistent with the version of the tiling in the header fixes, we should be fine. We just need to keep in mind that the tile_id (and some other keywords) may be wrong in the raw data.

@johndonor3, one more thing about header fixes: @JEMendezD reported a crash while reducing an exposure that requires this header fix:

https://github.com/sdss/lvmcore/blob/master/hdrfix/60273/lvmHdrFix-60273.yaml#L58

I think the issue is a wrong indentation level at line 58 (and down from there). Could you please take a look? Thanks!

@johndonor3
Copy link

Ah, actually I think it's the rest of the fixes in that file that are abnormal with an extra indentation ;) but my changes were later so I added the indentation to my changes. If it still doesn't work then all the indentations should probably be removed.

I think we can close this issue now?

@ajmejia
Copy link
Contributor

ajmejia commented Sep 2, 2024

Thanks @johndonor3! @JEMendezD has confirmed that this is not affecting his reductions anymore.

I think we're ready to close this one.

@ajmejia ajmejia closed this as completed Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants