search

esheldon / smatch

Code to match points on the sphere using the healpix scheme
Other
12 stars 3 forks source link

No docs describing the Matcher context manager #23

Open parejkoj opened 2 months ago

parejkoj commented 2 months ago

The readme discusses using smatch.match, but in LSST we're using the smatch.Matcher context manager, which does not have any mention in the readme here. Apparently those two interfaces use quite different code under the hood and one may have much better performance.

Please document the context manager interface in the package readme, so that users can be aware of it and make an informed choice about which interface to use.

beckermr commented 2 months ago

Thanks for the issue!

This repo is OSS and we will gladly+greatfully accept PRs with documentation, fixes, features, etc. If you are waiting for one of us to do the work, it may be a while before we get to it depending on how busy we are! 🚀

parejkoj commented 2 months ago

I don't know what the correct documentation should be for an interface I didn't even know existed until just a bit ago.

beckermr commented 2 months ago

Great! New users usually have the best perspectives on docs. If you have the time, by all means we'd welcome a contribution.

parejkoj commented 2 months ago

Cool, please explain to me what the differences between the interfaces are and how to use the context manager one so that I can understand it.

beckermr commented 2 months ago

Sure. The matcher class uses scipys kdtree classes to find nbrs on the sphere as opposed to using healpix. The context managers are there to help with memory management, tree construction+deletion, etc in the usual way. The best documentation we have for how to use them is the current test suite: https://github.com/esheldon/smatch/blob/master/smatch/tests/test_matcher.py. That has examples for how to call all of the methods and the code should be simple enough that it explains the outputs. Hope that helps!

parejkoj commented 2 months ago

What are the reasons to choose the Matcher context manager over just calling smatch.match? If I'm only making one cross-match call, should the larger or smaller catalog be the one passed to Matcher? Is it better to call query_knn or query_radius if I want to just find the closest matches?

beckermr commented 2 months ago

I actually don't know the answers to those questions. Maybe Eli has more insight? If you've tried things and have any experience, that'd be relevant too!