Test/coordinates core - #28
Open
mpound wants to merge 20 commits into
Open
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds slama.monitor.compute, the declarative recipe engine chosen in
docs/monitorsystem_writer_design.md: computations.json wires named,
registered Python functions to resolved SMAX input points, the engine
ticks in topological order (so aggregate-of-aggregate outputs see
this-tick values, not stale ones), and writes each result back to SMAX
as an ordinary point plus a "validity" metadata entry (validity is
computed, not natively stored by SMAX/Redis).
Structured like slama.fault (FaultSystem/FaultConfig/FaultNode): same
tick()/run_forever()/stop()/reload_config() shape, __each__ template
expansion, and load-time validation that fails fast on unknown
functions, undeclared outputs, or dependency cycles.
Built-in functions: worst_validity (severity-ranked, since Validity's
enum int order does not match severity order), count_true, count_valid,
min/max/mean/median_value, and sequence_validity (a stuck-timeout
trajectory check for state-machine string points, using a per-node
persistent ctx.state dict and ctx.params, per design doc §3.5/§6.1).
One real end-to-end example is wired: monitorsystem:array:antennas_online,
counting antenna:{1..8}:is_online. sequence_validity is unit-tested but
not wired into computations.json (no real tuning-state point exists in
smax.json yet).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All three shared one blind spot: every test fed plain Python values while production feeds smax.smax_data_types.Smax* instances. - MonitorPoint._state_machine_validity(): a real smax_pull() result is a SmaxStr, which (being an @DataClass with default eq=True) has __hash__ set to None, so a dict lookup on self.value directly raised TypeError. Coerce to str first. - ComputeConfig._resolve_pattern(): a colon segment containing '-' that isn't a numeric range (e.g. smax.json's literal "4K-plate", "a1-a2" segments) made _parse_index_set raise. Catch that locally and fall back to treating the segment as a literal, rather than loosening _parse_index_set's own contract (shared with slama.fault's __each__ expansion, where a malformed range is more likely a config typo). - engine._wrap(): passed the float wall-clock straight through as `timestamp`, but SmaxVarBase.timestamp is datetime | None and MonitorPoint.time does Time(self._smax_result.timestamp) -- astropy rejects a bare float. Convert to an aware datetime. Also narrowed the bare `except Exception` around the resulting mp.time read in _resolve_one(), which would otherwise convert this exact class of bug into a silent, permanent INVALID_NO_DATA. Also: warn (rather than silently ignore) when a computation sets the reserved-but-unconsulted per-entry interval_s; and worst_validity now writes a severity-ranked score instead of int(Validity), since the whole reason _SEVERITY_ORDER exists is that the enum's own ordinal isn't severity-ordered -- writing it as a value would have scrambled any later numeric/threshold comparison on that point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
output now accepts a dict of role-name -> canonical-name for entries whose function naturally computes several related points in one pass (e.g. min/max), avoiding a double call and keeping the function reusable under __each__ expansion. Also hardens tick() with per-node error isolation: any exception, including a multi-output key mismatch, now only invalidates that node instead of aborting the whole tick. Folds the design into docs/monitorsystem_writer_design.md §8. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers every function and method in engine.py and computeconfig.py that lacked one or had only a one-line summary, including private helpers -- Parameters/Returns/Raises added throughout. No behavior change. computenode.py already had full docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nction for computing sun angle
- coordinates/core.py: sun_distance() referenced angular_separation without importing it (NameError on call); switched hasattr(x, "unit") checks to isinstance(x, u.Quantity) since a real SMAX value (SmaxFloat etc.) carries its own unrelated .unit metadata attribute, always making hasattr() true and skipping the degree conversion. Same fix applied to sun_distance_from_coord. Length check now uses np.size() so scalar (single-antenna) calls work, not just arrays. - functions.py: replaced the 32-input/8-output sun_distance_degrees (which had an output-shape mismatch against the engine's contract, an off-by-one slice, and passed ResolvedInput objects instead of raw values) with a single-antenna, dict-input version paired with a __each__-expanded entry per antenna instead. - computations.json / smax.json: sun_distance entries/points declared per-antenna via __each__, with warn_low=45, err_low=40 degrees per Marc. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers sma_location/SMA singleton, fake_sma_antpos, ra2ha, obsfreq/ restfreq, eq2hor/hor2eq (including round trips), solar_coordinate, sun_altaz, and sun_distance. Round trips and geometric special cases (zenith declination, pointing-at-the-sun) used where possible instead of hardcoded reference numbers, so tests stay meaningful if astropy's ephemeris/IERS data changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
feature/monitor-compute-engine renamed the original 3-arg SkyCoord-based sun_distance() to sun_distance_from_coord() and added a new 4-arg (sunaz, sunel, antaz, antel) sun_distance(), used by the monitor.compute sun_distance function. Split the old TestSunDistance class accordingly and added direct coverage for the new signature. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test code for new coordinates module.
(will fail until PR #25 is merged)