Jim Bosch suggested that a context manager interface might be a good way to make progress on this. This is a time-consuming, but a more-principled solution.
https://lsstc.slack.com/archives/C2JPMCF5X/p1619627754121800?thread_ts=1619626296.119600&cid=C2JPMCF5X

If we want a faster algorithm to check that the children are ordered by parent, can't we just check that the parents are ordered too? In other words in python

np.all(catalog["id"][1:]-catalog["id"][:-1] > 0)

np.all(catalog["parent"][1:]-catalog["id"][:-1] >= 0)

That way you could run it one for all of the parents.

Checking if the catalog is ordered or not before calling `getChildren` method was my initial thought as well, but then we will have to make that check everywhere before this method is invoked (which was only a handful number of times). But Jim's concern was that there may be other functions that still assume that the catalog is sorted and might fail. Either way, we need to remove this check within the `getChildren` method here, which will force a check given a parent: https://github.com/lsst/afw/blob/f21da11f85345907a2a1784b5f2f45b65e25ac04/python/lsst/afw/table/_source.py#L61

To motivate this case further, I did a timing test on a catalog (sorted) with ~2000 parents. Getting their children by running the `getChildren` method on each of the parent consistently took > 1s, whereas skipping that check runs in ~0.05s. So I think this is an optimization worth making.

Edit: Adding a one-time check did not make any significant difference by itself.

Jenkins run: -https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34145/pipeline-
https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/34154/pipeline

There are two PRs for this ticket. Jira is picking up only one.

afw: https://github.com/lsst/afw/pull/585
meas_base: https://github.com/lsst/meas_base/pull/174

Thanks for pointing out the missing PR associated with the ticket. Everything looks good, it's a great approach.

Thanks for the review, Fred! I'm glad you like the generator-based approach. For the record, the other approaches I considered were:

1. having an attribute `_sorted` associated with each catalog, which has to be set every time a record is inserted, or catalogs are merged. Instead of running `isSorted` method, one can check the attribute alone.
2. Context Manager interface that Jim suggested.

However, both those methods seemed like over-engineering. The current implementation seemed close to ideal, but not perfect since it still checks if the catalog is sorted once more than necessary - once when fetching the parents and once when fetching the children. That's a redundancy that we can afford to have easily.

Putting this back In Progress for now; the merges caused ci_hsc failures, and I reverted them Saturday morning (didn't have time to investigate further, and didn't want to ask anyone else to over the weekend).  I imagine we'll want to create a new ticket for the second attempt, move the commits to that branch, and then just mark this one as Invalid, but that can wait for next week.

Apologies! That's the first time that happened to me. I'll have a look at the logs by Monday. I have a rough idea what might have gone wrong.

I think I've fixed it now. There was a logic bug in the `subset` method `references.py` file that caused children sources that are outside of the bounding box also to be returned. The fix is fairly simple and pushed it in the same branch. Since the fix is simple, I don't know if it can be in the same ticket or has to come from another ticket necessarily because the PR was reverted.

Weirdly, ci_hsc_gen3 doesn't have build issues, only ci_hsc_gen2 does. Is this a concern?

This happens rarely enough that we don't have a policy (AFAIK) for whether to make it another ticket, but I would recommend another ticket just to avoid various kinds of clashes complicating things.

There are some things ci_hsc_gen2 does that ci_hsc_gen3 does not, and they take different code paths in much of the I/O.  I wouldn't have expected them to differ on this ticket, but that's not a strong expectation and I wouldn't be able to say more without looking much more closely.

Alright. Marking this as Won't Fix now and will create a new PR with the patch included in DM-30105.

To conclude the gen2/gen3 differences in this ticket: in forcedPhotCcd.py, ci_hsc_gen2 gets the `refCat` by calling the `fetchReferences` method from `runDataRef` whereas, ci_hsc_gen3 gets the same by calling `mergeAndFilterReferences` method in `runQuantum`. Only the first method takes the code path that contained the bug, so it is perfectly reasonable that ci_hsc_gen3 build passed but not ci_hsc_gen2.