legacysurvey / legacypipe

Image reduction pipeline for the DESI Legacy Imaging Surveys, using the Tractor framework
BSD 3-Clause "New" or "Revised" License
37 stars 22 forks source link

Update star-clusters-supplemental to support pa/ba. #701

Closed schlafly closed 2 years ago

schlafly commented 2 years ago

Add LMC and SMC to star-clusters-supplemental. Fix build-cluster-catalog to reflect changes to source NGC catalog.

This was more annoying than I was hoping.

The OpenNGC catalog format appears to have changed, so build-cluster-catalog.py file would not create a new cluster catalog. We could have either pinned to a specific version of that file or updated the code to make it work. I chose to update the code. I didn't like the renaming of all of the columns in that catalog, and so I updated the code to use the original names.

I added column headings to the star-clusters-supplemental file so that we have at least a modicum of documentation there. I removed the code adding Fornax and Sculptor and instead added these to the star-clusters-supplemental file with the LMC and NGC.

Going through this, I realized that the reason we failed to unmask M67 / NGC 2682 is that legacypipe does not actually care about star-clusters-supplemental, and instead only cares about NGC-star-clusters.fits. We need to rebuild the latter file using build-cluster-catalog to have any changes in the lists be reflected in the processing. This PR includes a rebuild NGC-star-clusters.fits.

I compared the new and old NGC-star-clusters.fits files. They are not the same. Some of the differences are expected: the LMC and SMC are now included, and NGC 2682 is now excluded. Additionally, the radii of Fornax and Sculptor changed slightly because I only propagated 3 significant digits into the star-clusters-supplemental file; fine.

On top of these expected changes, there are unexpected changes. the new catalog also contains IC4670. It looks like this is out of our footprint, so we probably don't care. It's in the source NGC catalog, so I suspect its type changed to allow it to be included? I haven't tracked this further. We also additionally exclude NGC2420, which we used to include. That's this one https://www.legacysurvey.org/viewer?ra=114.6017&dec=21.5695&layer=ls-dr9&zoom=12&GCs-PNe which I could imagine either including or excluding. This object is excluded because the source catalog classifies it as an OCl, but only GCl and PN are kept. Maybe this is also a change in the source NGC catalog.

@moustakas , I'm going to push this back to you.