Structured array source detection catalogs #987

bmorris3 · 2023-11-10T18:40:47Z

Source Detection produces a 2D ndarray of source IDs, centroids, and fluxes, to use as inputs in tweakreg. @schlafly and I discussed in #841 that we could use a self-documenting and more scalable data structure instead if additional Source Detection results are useful for tweakreg or elsewhere.

This PR swaps the unlabeled 2D ndarray for a structured array, and adds some helper functions for converting between ndarrays, structured arrays, and astropy tables.

Resolves RCAL-709

Checklist

added entry in CHANGES.rst under the corresponding subsection
updated relevant tests
updated relevant documentation
updated relevant milestone(s)
added relevant label(s)
ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

codecov · 2023-11-10T19:00:31Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (9cf7c04) 73.53% compared to head (df1b05e) 73.55%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
romancal/source_detection/source_detection_step.py	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #987      +/-   ##
==========================================
+ Coverage   73.53%   73.55%   +0.01%     
==========================================
  Files         103      103              
  Lines        6704     6711       +7     
==========================================
+ Hits         4930     4936       +6     
- Misses       1774     1775       +1

Flag	Coverage Δ		*Carryforward flag
nightly	`63.26% <ø> (-0.01%)`	⬇️	Carriedforward from 9cf7c04

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

schlafly

This looks good. I might take it a bit farther and aim to just make 'sources' the catalog (don't bother trimming out columns unless we really think daofind is giving us useless stuff?), and then have the fit_psf branch just ~join the psf_photometry_table onto the sources table + overwrite the x & y so that they get used by tweakreg? Does the psf_photometry_table already include all of the content of the daofind sources table?

It's unclear to me that you actually use or need ndarrays_to_recarrays or recarray_to_ndarrays. I think we can get away without the whole basic_utils.py file, and just rely on Table(ndarray) and Table(ndarray).as_array() to handle all of the conversions between structured arrays and Tables?

schlafly · 2023-11-10T18:53:07Z

romancal/lib/basic_utils.py

+    return Table(data, names=names)
+
+
+def astropy_table_to_recarray(x):


I feel like I usually use https://docs.astropy.org/en/stable/api/astropy.table.Table.html#astropy.table.Table.as_array
?

This function converts specifically to a structured array.

schlafly · 2023-11-10T18:56:01Z

romancal/lib/basic_utils.py

+    return np.asarray(x.astype(astype).view(to_dtype).reshape((-1, len(names))))
+
+
+def recarray_to_astropy_table(x):


I think you can just do Table(ndarray).

Thanks you're right! Resolved.

bmorris3 · 2023-11-10T19:44:49Z

I might take it a bit farther and aim to just make 'sources' the catalog (don't bother trimming out columns unless we really think daofind is giving us useless stuff?), and then have the fit_psf branch just ~join the psf_photometry_table onto the sources table + overwrite the x & y so that they get used by tweakreg?

That's almost what's done now. I left the DAO trimming in place, as-is. When fit_psf=True, the photometry results table is saved as the catalog, but renames the centroid columns to be what tweakreg expects. The DAO results are the initial guesses to the PSF fit, so those centroids and fluxes are preserved in the resulting table as x_init, y_init, flux_init.

Does the psf_photometry_table already include all of the content of the daofind sources table?

Not all of the content, but I'd argue that the parts that are relevant are preserved.

schlafly · 2023-11-13T17:30:49Z

romancal/lib/basic_utils.py

@@ -103,3 +103,45 @@ def __exit__(self, et, ev, tb):
        if self.handler and self.close:
            self.handler.close()
        # implicit return of None => don't swallow exceptions
+
+
+def ndarrays_to_recarray(arrays, names):


Am I missing it, or do we only use this and its inverse in tests? Can we get rid of this module and its tests and just rely on the library conversions?

It's only used in tests at the moment, that's correct. I left this in the PR until chatting with @mairanteodoro to confirm that we won't ever need to read and convert the old catalogs that are 2D ndarrays.

@bmorris3 I think we should be good. Thanks!

mairanteodoro

Looks good to me!

mairanteodoro · 2023-11-13T21:35:31Z

romancal/lib/basic_utils.py

@@ -103,3 +103,45 @@ def __exit__(self, et, ev, tb):
        if self.handler and self.close:
            self.handler.close()
        # implicit return of None => don't swallow exceptions
+
+
+def ndarrays_to_recarray(arrays, names):


@bmorris3 I think we should be good. Thanks!

bmorris3 requested a review from a team as a code owner November 10, 2023 18:40

github-actions bot added the testing label Nov 10, 2023

bmorris3 requested a review from mairanteodoro November 10, 2023 18:41

bmorris3 force-pushed the structured-array-catalogs branch from 5764aa0 to 6f58a89 Compare November 10, 2023 18:44

bmorris3 force-pushed the structured-array-catalogs branch from 6569baa to 32d5af7 Compare November 10, 2023 19:08

bmorris3 requested a review from schlafly November 10, 2023 19:09

schlafly reviewed Nov 10, 2023

View reviewed changes

bmorris3 force-pushed the structured-array-catalogs branch 2 times, most recently from 588e40d to 95e3bdc Compare November 13, 2023 17:25

bmorris3 requested a review from schlafly November 13, 2023 17:25

schlafly reviewed Nov 13, 2023

View reviewed changes

bmorris3 mentioned this pull request Nov 13, 2023

Source detection+PSF docs updates #984

Merged

mairanteodoro reviewed Nov 13, 2023

View reviewed changes

mairanteodoro approved these changes Nov 13, 2023

View reviewed changes

bmorris3 force-pushed the structured-array-catalogs branch from 95e3bdc to 6b8de32 Compare November 14, 2023 02:14

bmorris3 added 2 commits November 13, 2023 21:14

storing tweakreg results in structured arrays

5e7e042

structured array Source Detection output/tweakreg input

df1b05e

bmorris3 force-pushed the structured-array-catalogs branch from 6b8de32 to df1b05e Compare November 14, 2023 02:14

bmorris3 merged commit 50219a1 into spacetelescope:main Nov 14, 2023
28 checks passed

bmorris3 mentioned this pull request Nov 27, 2023

Adding a basic source catalog from Source Detection Step #1029

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Structured array source detection catalogs #987

Structured array source detection catalogs #987

bmorris3 commented Nov 10, 2023

codecov bot commented Nov 10, 2023 •

edited

Loading

schlafly left a comment

schlafly Nov 10, 2023

bmorris3 Nov 10, 2023

bmorris3 Nov 13, 2023

schlafly Nov 10, 2023

bmorris3 Nov 13, 2023

bmorris3 commented Nov 10, 2023 •

edited

Loading

schlafly Nov 13, 2023

bmorris3 Nov 13, 2023

mairanteodoro Nov 13, 2023

mairanteodoro left a comment

mairanteodoro Nov 13, 2023

		return Table(data, names=names)


		def astropy_table_to_recarray(x):

		return np.asarray(x.astype(astype).view(to_dtype).reshape((-1, len(names))))


		def recarray_to_astropy_table(x):

Structured array source detection catalogs #987

Structured array source detection catalogs #987

Conversation

bmorris3 commented Nov 10, 2023

codecov bot commented Nov 10, 2023 • edited Loading

Codecov Report

schlafly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmorris3 commented Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mairanteodoro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 10, 2023 •

edited

Loading

bmorris3 commented Nov 10, 2023 •

edited

Loading