MOC-based query#401
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for MOC-based spatial alert searches (including HEALPix skymap thresholding) as an alternative/supplement to cone-search, along with supporting utilities, API wiring, and indexing.
Changes:
- Introduces
utils::mochelpers to parse MOC/skymap FITS, test containment, and build a cone-cover approximation for MongoDB geospatial queries. - Adds a new Babamul API endpoint
POST /surveys/{survey}/alerts/moc-searchand wires it into routing + OpenAPI. - Adds a MongoDB index on
candidate.jdto speed up time-window filtering used by MOC searches.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/mod.rs |
Exposes the new moc utility module. |
src/utils/moc.rs |
Implements MOC/skymap FITS parsing, containment, cone covering, and unit tests. |
src/utils/db.rs |
Adds candidate.jd index creation for alerts collections. |
src/enrichment/lsst.rs |
Refactors LSST footprint MOC loading/containment to use shared utils::moc. |
src/bin/api.rs |
Increases JSON payload size limit under /babamul and registers the new route. |
src/api/routes/babamul/surveys/mod.rs |
Re-exports moc_search_alerts. |
src/api/routes/babamul/surveys/alerts.rs |
Adds the MOC search endpoint implementation. |
src/api/docs.rs |
Includes the new endpoint in OpenAPI docs. |
data/glg_healpix_all_bn200524211.fits |
Adds a (LFS) skymap FITS fixture used by MOC tests. |
tests/data/alerts/ztf/ZTF20abbiixp_last_alert.json |
Adds a ZTF alert JSON fixture (currently unused). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
| // Larger JSON payload limit for MOC/skymap uploads (100 MB) | ||
| .app_data(web::JsonConfig::default().limit(104_857_600)) |
There was a problem hiding this comment.
I've never seen a MOC that big????????? This change seems weird to me.
There was a problem hiding this comment.
and even then, I would not want this parameter on all other endpoints
| // create an index on candidate.jd for temporal range queries (MOC search, etc.) | ||
| let jd_index = doc! { | ||
| "candidate.jd": 1, | ||
| }; | ||
| create_index(&alerts_collection, jd_index, false).await?; | ||
|
|
There was a problem hiding this comment.
we may want to just start adding some proper indexes on the parameters we care about. A query JUST on JD (MongoDB indexes should be compound as much as possible, since 2 different indexes aren't used together like they may in postgres) is very unlikely. I would suggest considering other fields we use on these queries.
There was a problem hiding this comment.
I mean here for example I see you clearly run cone searches. So at minimum, having JD and coordinates is a must.
| #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, ToSchema)] | ||
| struct AlertsMocSearchQuery { | ||
| /// Base64-encoded MOC FITS file (exactly one of moc_fits_base64 or skymap_fits_base64 required) | ||
| moc_fits_base64: Option<String>, | ||
| /// Base64-encoded HEALPix skymap FITS file | ||
| skymap_fits_base64: Option<String>, | ||
| /// Credible level for skymap thresholding (optional, defaults to 0.9 if omitted when skymap_fits_base64 is provided) | ||
| credible_level: Option<f64>, | ||
| /// Start of time window (required, Julian Date) | ||
| start_jd: f64, | ||
| /// End of time window (required, Julian Date, max 7 days after start_jd) | ||
| end_jd: f64, | ||
| min_magpsf: Option<f64>, | ||
| max_magpsf: Option<f64>, | ||
| #[serde(alias = "min_reliability")] | ||
| min_drb: Option<f64>, | ||
| #[serde(alias = "max_reliability")] | ||
| max_drb: Option<f64>, | ||
| is_rock: Option<bool>, | ||
| is_star: Option<bool>, | ||
| is_near_brightstar: Option<bool>, | ||
| is_stationary: Option<bool>, |
There was a problem hiding this comment.
To echo my other comment on the index: why not just have a compound index on all these parameters so we know for SURE that query is covered???
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
|
Throughput results (
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
81506d8 to
1e643c5
Compare
|
Throughput results (
Baseline run: |
|
Throughput results (
Baseline run: |
|
Throughput results (
Baseline run: |
This PR adds a MOC-based query to supplement the cone search option