Cross matches

[javierggt]

This PR is a refactor of the cross-match methods, partly to address requests in issue #15 and partly to fix a few things.

The things that changed upon @taldcroft request:

• Cross_match.cross_match changed so one can customize the query.
• Docstrings much improved.
• added methods to exclude regions/targets, and optionally exclude regions in cross_match

Other cleanup:

• Removed older functions that are not used.
• The cross_match name is now used (it defaults to astromon_21).
• Added the radial cut for grating observations. Until now it was the same as non-grating.

Testing

• [x] Unit tests in mac
• [x] All the example code in the docs. Before running the last example in the quick start, I did the following, which creates a test file:

  from astromon.tests import utils
  utils.create_h5('some_file.h5')

  NOTE: the observation code examples in the docs only run on HEAD and if you have access to arc5gl.

• [x] Functional test (running the production script). I did the following on HEAD and verified that the cross-matches are the same as before:

  python -m astromon.scripts.get_cat_obs_data --workdir work --obsid 26266 --db-file pr-17.h5

  and

  python -m astromon.scripts.get_cat_obs_data --workdir work --start 2022-01-04 --stop 2022-01-05 --db-file pr-17-2.h5

  and this one only writes xray_src and obs, the rest are empty

  python -m astromon.scripts.get_cat_obs_data --workdir work --obsid 26249 --db-file pr-17-3.h5

[taldcroft]

Not quite in this PR, but this code got my attention. It is making a view of a subset of a Table and then sorting within that view. Apparently that works, but it still strikes me as possibly sketchy. It is also a lot slower than it needs to be because of making lots of tables and sorting columns that don't need attention.

    matches = matches.group_by('obsid')
    for g in matches.groups:
        g.sort(['cat_order', 'dr'])
    result = table.vstack([g[0] for g in matches.groups])

This should be orders of magnitude faster and equivalent [EDIT: confirmed]:

    mg = matches.group_by('obsid')
    indices = mg.groups.indices
    idxs = []
    for i0, i1 in zip(indices[:-1], indices[1:]):
        idxs_sort = np.lexsort((mg['dr'][i0:i1], mg['cat_order'][i0:i1]))
        idxs.append(i0 + idxs_sort[0])
    result = mg[idxs]

[javierggt]

Not quite in this PR...

I could make an issue with this and follow-up after this PR. I will check.

[taldcroft]

Digging further on the point about the cross match code above, two things:

1. It is actually broken and I confirmed that the sources returned don't adhere to the expected catalog priority ordering. For instance for obsid 1520 xid=6, there are matches for SDSS and 2MASS. Using catalogs=['ICRS', 'Tycho2', '2MASS', 'SDSS', 'USNO-B1.0'], should choose 2MASS. My original get_xcorr gives the 2MASS match, but the result from this branch is the SDSS one.

[EDIT] I think this was a different bug unrelated to the cross-match code in question.

4. It is extremely slow in a apples-to-apples comparison with my original code. The line below is intended to do no filtering at all (to match my original get_xcorr(xray_src, cat_src, obs) function). The code below takes about 3 minutes to complete while the original get_xcorr finishes in 4 seconds.

   xcorr = cross_match.compute_cross_matches(
   catalogs=['ICRS', 'Tycho2', '2MASS', 'SDSS', 'USNO-B1.0'],
   dr=10000,
   snr=-10000,
   r_angle=10000,
   r_angle_grating=10000,
   near_neighbor_dist=-10000,
   )

[taldcroft]

I fixed the discrepancies between my original API code and this version with ff34529.

[taldcroft]

@javierggt - this PR is basically fine at this point except for the minor comments I made that are all easily fixed. If you just take care of those then we can merge this one.

[javierggt]

I made changes that I believe address @taldcroft's comments.

I also made a couple new commits:

• removed an unused script
• changed the argument orders in db.save to be consistent
• updated test data

[taldcroft]

I pushed a commit so the narrative docs include the information that future-me will need.

[javierggt]

I committed a fix for a failure I got, which I believe traces back to changing the return values of empty tables. I didn't spend much time checking why, but the bottomline is I add some extra columns to empty tables so the result's type is consistent with the case of non-empty tables, and downstream code can handle it.

I ran the tests above and check that the output is consistent with expectations.

[javierggt]

I fixed two typos and will squash in a moment.

[taldcroft]

:tada: