askap-vast / vast-pipeline

This repository holds the code of the Radio Transient detection pipeline for the VAST project.
https://vast-survey.org/vast-pipeline/
MIT License
8 stars 3 forks source link

compute ideal source coverage with astropy xmatch #555

Closed marxide closed 3 years ago

marxide commented 3 years ago

Previous implementation required the Cartesian product of every source and sky region be held in memory. Fixes #554.

marxide commented 3 years ago

You've just replaced that part with an astropy crossmatch search around sky, right? Definitely a leaner way to go, thanks.

Yes. Instead of making a DataFrame of each source and sky region combination with their separations then filtering by the separation, it does the same with the astropy search around sky function. The separation threshold needs to be different per sky region, so I use the maximum first, then filter again properly later.

On a side note, from memory, I had speed issues when performing separation calculations using astropy in conjunction with a pandas.apply() and/or vectorising things - that's pretty much the only reason that on_sky_sep function exists.

I didn't test if this is any faster or slower. It might be slower, but I think sacrificing speed in return for avoiding OOM errors is preferable!

ajstewart commented 3 years ago

On a side note, from memory, I had speed issues when performing separation calculations using astropy in conjunction with a pandas.apply() and/or vectorising things - that's pretty much the only reason that on_sky_sep function exists.

I didn't test if this is any faster or slower. It might be slower, but I think sacrificing speed in return for avoiding OOM errors is preferable!

For sure! Just wanted to give context on why that function is there in the first place.