Skip to content

Color estimation from observations - #202

Open
HoodedCrow wants to merge 13 commits into
B612-Asteroid-Institute:mainfrom
HoodedCrow:color
Open

Color estimation from observations#202
HoodedCrow wants to merge 13 commits into
B612-Asteroid-Institute:mainfrom
HoodedCrow:color

Conversation

@HoodedCrow

Copy link
Copy Markdown
Contributor

Computes H_{g,i,r,u} and differences g-r, g-i, and r-i from MPC observations and orbits. Checked against selected well-matched values from the rotational period paper. Note that the color estimation doesn't require rotational period and assumes g(t)=0 instead.

Supported phi(alpha): c1c2 (polynomial fit), GH12* (splines), GH (old tan(a/2) model already in photometry)

ntellis added a commit that referenced this pull request Jun 29, 2026
…6.5.1 (CI unblock)

CI bumped black to 26.5.1, which reformats two pre-existing files that predate
this PR (not rotation-period code; confirmed absent from this PR's diff vs main,
and main's own CI is currently red on the same two files). Apply the same black
26.5.1 reformat that the other open PRs (#201, #202) carry, so PR #200's `lint`
job goes green. Pure formatting, no behavior change; isort + ruff already clean.

(The separate `test (3.12)` failure is test_mpc_observatory_code in observers/ --
a flaky/environment-dependent ephemeris result, unrelated to this PR, which
touches no observers code; it clears on a CI re-run.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@akoumjian

Copy link
Copy Markdown
Contributor

I like how this is setup. We should plug in our existing filter band implementations though instead of using generic g, i, r, u. If you look you'll find utilities around both loading response curves for instrument specific filters, and then also utilities that map (observatory, band) combinations to canonical filter ids (also includes MPC labels mapping as well, e.g. ATLAS, Ao -> ATLAS o).

Here was some ai feedback from gpt-5.5:

 ### Main concerns

 1. File name typo
     - The module is named color_detemination.py, missing the “r” in “determination”.
     - That becomes public API if merged. I’d fix before merge.
 2. Not exported from adam_core.photometry
     - Existing photometry public functions are exported in photometry/__init__.py.
     - This PR does not add estimate_colors or ColorFit there, so discovery/import ergonomics are poor.
 3. Runtime dependency issue
     - New production code imports mpcq, but mpcq appears to be only a test dependency in pyproject.toml, and only for Python <3.13.
     - If this is intended as public runtime functionality, mpcq needs to be a real dependency or this feature should be behind an optional extra/lazy import.
 4. Raw band handling is too narrow
     - The estimator only recognizes literal "g", "i", "r", "u".
     - The repo already has canonical bandpass mapping utilities, but this code does not use them.
     - The fixture generator even computes filter_id, then the tests ignore it. That feels like an unfinished integration point.
 5. Unconstrained phase parameters
     - G / G12* are fit with no physical bounds.
     - I’d use bounds or at least validate/report out-of-range fits.
 6. Outlier rejection is mislabeled
     - The comment says “9-sigma”, but the code uses squared residuals and compares to 9 * sigma2, which corresponds to roughly 3-sigma, not 9-sigma.
 7. Fit diagnostics are thin
     - Output does not include fitted phase parameter, uncertainties, chi-square, rank/DOF, or convergence status.
     - For scientific use, that makes it hard to judge reliability.
 8. Library code uses print
     - Diagnostic messages should use logging or structured return metadata, not print from library code.
 9. Tests are mostly fixture/end-to-end
     - Useful, but I’d add small synthetic tests with known colors/phase behavior.
     - hg12star.py should have direct unit tests against reference table values.
 10. No actual asteroid-class matching
 - The file header says “matching the color mix to asteroid class,” but the implementation only estimates colors. No taxonomy/classification is implemented.

@ntellis

ntellis commented Jun 30, 2026

Copy link
Copy Markdown
Member

I'll echo Alec's comments about the filter band implementations. Existing inverse photometry maps exposures through map_to_canonical_filter_bands before modeling at absolute_magnitude.py (line 154)

I also have a PR out for rotation. It has a few related color components, so as not to overlap we might want to make these changes:

  • Replace manual distance reduction mag - 5 * np.log10(r * delta) with the new shared reduced_magnitude(...) from adam_core.photometry.lightcurve / exported adam_core.photometry.reduced_magnitude.
  • Delete _hg_correction(...) and use the shared hg_phase_correction(...) now exported from adam_core.photometry / defined in magnitude_common.py. That keeps Bowell H-G behavior single-sourced with magnitude.py and the rotation pipeline.
  • (Maybe) treat the new rotation lightcurve helpers as the integration point: color fitting could operate on the same reduced-magnitude / geometry columns that rotation uses, rather than owning a parallel reduction path.

Assumes same band letter is the same channel for different sources
(e.g. SDSS_g vs LSST_g).
A color channel can pool observations from different same-letter filters
(e.g. SDSS_g and LSST_g for "g"). Merging them directly biases the
per-channel H by the inter-system color term. A new parameter (default
None, so noop) allows to use color correction per STN code and spectrum
type (composition). The difference between compositions is small, so
it is still okay to guess composition before colors are determined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants