desihub / desidatamodel

The DESI data model.
BSD 3-Clause "New" or "Revised" License
4 stars 10 forks source link

EDR/fuji: Equivalent widths in emline files off by (1+z)^2 factor #180

Closed araichoor closed 1 year ago

araichoor commented 1 year ago

The slack message: https://desisurvey.slack.com/archives/C2AT2NEHY/p1691611194432619 reminded me that the fuji emline files actually may still have the equivalent widths wrong by a (1+z)^2 factor. See https://github.com/desihub/desispec/issues/1876.

The PR correcting the code bug has been merged in Oct. 2022 (https://github.com/desihub/desispec/pull/1878), so after the fuji files have been generated (~Feb.-May 2022), right?

Should we add a warning in the notes in https://github.com/desihub/desidatamodel/blob/main/doc/DESI_SPECTRO_REDUX/SPECPROD/tiles/GROUPTYPE/TILEID/GROUPID/emline-SPECTROGRAPH-TILEID-GROUPID.rst?

And also add a bullet here: https://data.desi.lbl.gov/doc/releases/edr/known-issues/?

Pinging @sbailey.

For fastspecfit, I believe the code bug has been fixed, and the data re-processed, right? (@moustakas).

sbailey commented 1 year ago

Thanks for remembering that, @araichoor . Your date and code accounting looks correct, but please also double check by running the latest emlinefit on Fuji spectra and comparing to Fuji emline files, or otherwise comparing Fuji and Iron emline files. If indeed Fuji is off by (1+z)^2, please update the datamodel and edr/known-issues docs that you mentioned. Thanks!

araichoor commented 1 year ago

thanks for the suggestion. here s one quick check iron vs. fuji on one tile, confirming that:

tmp

code snippet:

from glob import glob
import fitsio
import numpy as np
from astropy.table import Table
from matplotlib import gridspec

tileid, lastnight = 80615, 20201216
fns = sorted(glob("/global/cfs/cdirs/desi/spectro/redux/fuji/tiles/cumulative/{}/{}/emline-?-{}-thru{}.fits".format(tileid, lastnight, tileid, lastnight)))
# read
fuji = vstack([Table(fitsio.read(fn, "EMLINEFIT")) for fn in fns])
iron = vstack([Table(fitsio.read(fn.replace("fuji", "iron"), "EMLINEFIT")) for fn in fns])
assert np.all(fuji["TARGETID"] == iron["TARGETID"])
# restrict to TGT
sel = fuji["OBJTYPE"] == "TGT"
fuji, iron = fuji[sel], iron[sel]
# grab computed emission lines
emnames = fits.getheader(fns[0], "EMLINEFIT")["EMNAMES"].split(",")

# plot
fig, ax = plt.subplots()
xlim, ylim = (-0.1, 2), (-2, 10)
for emname in emnames:
    key = "{}_EW".format(emname)
    ax.scatter(fuji["Z"], fuji[key] / iron[key], s=5, alpha=0.1, label=emname)
xs = np.linspace(xlim[0], xlim[1], 100)
ax.plot(xs, (1 + xs) ** 2, color="k", ls="--", zorder=1, label="y = (1 + x) ** 2")
ax.legend(loc=2)
ax.set_title("TILEID={}, LASTNIGHT={}, OBTYPE=TGT".format(tileid, lastnight))
ax.set_xlabel("Fuji(Z)")
ax.set_ylabel("EW_fuji / EW_iron")
ax.grid()
ax.set_xlim(xlim)
ax.set_ylim(ylim)
#
plt.savefig("tmp.png", bbox_inches="tight")
plt.close()
araichoor commented 1 year ago

I can make a PR to request update on https://github.com/desihub/desidatamodel/blob/main/doc/DESI_SPECTRO_REDUX/SPECPROD/tiles/GROUPTYPE/TILEID/GROUPID/emline-SPECTROGRAPH-TILEID-GROUPID.rst.

But what is the procedure to request update on https://data.desi.lbl.gov/doc/releases/edr/known-issues/?

sbailey commented 1 year ago

Thanks for the check! In the datamodel PR, please mention that this is a bug in Fuji/EDR but has been fixed for Iron/DR1. For https://data.desi.lbl.gov/doc/releases/edr/known-issues/, that is updated via a PR to github.com/desihub/desidatadocs. After that is merged, Angela, Ben, or myself can re-deploy the website.

weaverba137 commented 1 year ago

@sbailey, was it also fixed in guadalupe?

araichoor commented 1 year ago

right, I forgot about guadalupe. guadalupe was run at the same time as fuji, ie ~Feb.-May 2022 (and with the same code version, I think). so I expect that the bug is also present in guadalupe.

sbailey commented 1 year ago

yes, this is also an issue with guadalupe.

weaverba137 commented 1 year ago

This was closed by #181