Repository navigation
Add available_engines() and active_implementation() introspection functions - #14
Jim Garrison (garrison) wants to merge 3 commits into
Conversation
These are the two functions anticipated when enable_engine became a free function: the argument for that shape was that a domain author should not have to re-export each new user-facing function, so these follow it. available_engines(domain) returns a sorted tuple of the engine names that can be enabled. It unions the domain's engine registry with the names advertised through the coheriq.engines.<domain> entry point group, because enable_engine accepts either. Advertised plugins are deliberately not imported to answer the question, so a listed name means enable_engine will attempt it, not that it is guaranteed to load. active_engine(domain) returns the name of the enabled engine, or None. None covers both a domain that has not resolved its implementation yet and one already frozen on its defaults: "no engine is active" is equally true of both, and collapsing them keeps the return type str | None. Neither function resolves or freezes the implementation. This is the deliberate contrast with calling a candidate: asking which engine is active must not commit the domain to an answer, since that would make an introspection call silently foreclose enabling an engine afterwards. A sorted tuple rather than a set: these results are usually shown to a person, and set iteration order for strings varies between processes, which would make documentation examples and error messages non-reproducible. Also factors the domain-or-name coercion out of enable_engine into a shared _resolve_domain helper, now that three public functions accept that same argument. Adds a release note; this is the first note in releasenotes/notes/. Assisted-by: Claude Opus 5
The previous signature returned None in two distinct situations: the domain had not resolved an implementation yet, and the domain had resolved to its own default implementation. Those are opposite answers from the caller's point of view. In the first case an engine can still be enabled; in the second it is permanently too late. Overloading None meant a caller could not tell the actionable state from the settled one. The function now returns None only for "not decided yet", and the new REFERENCE constant (the string "reference") when the domain has resolved to the implementation it provides itself. Any other string is an engine name. Since None no longer conflates the two, a None result cannot mislead a caller into thinking the decision is still open when it is closed. Naming the resolved default "reference" follows the usual HPC sense of a reference implementation, and it is what makes the rename appropriate: the function reports which implementation is in effect, and the domain's own implementation is one of the possible answers, so "engine" was too narrow a word for the return value. Because "reference" is now a possible result, it can no longer also be an engine name; AccelerationEngine rejects it via a _RESERVED_ENGINE_NAMES denylist. The denylist is a compatibility hazard for any engine already using a name added to it, so it is kept deliberately small and the comment says so. active_implementation() classifies the domain states explicitly rather than treating "not ENGINE_ENABLED" as the undecided case, so that adding a state later forces a deliberate decision about how it is reported. Assisted-by: Claude Opus 5
Coverage Report for CI Build 36934656488Coverage increased (+3.0%) to 89.076%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
| * :data:`~coheriq.REFERENCE` (the string ``"reference"``) -- the domain has | ||
| resolved to its own reference implementation. No engine is active, and it | ||
| is too late to enable one. | ||
| * any other string -- the name of the engine that is active. |
There was a problem hiding this comment.
| * any other string -- the name of the engine that is active. | |
| * any other non-empty string -- the name of the engine that is active. |
| Unlike calling an acceleration candidate, this does not resolve or freeze the | ||
| implementation. Asking what is active never commits the domain to an answer, | ||
| so a ``None`` result does not become stale merely by being observed. |
There was a problem hiding this comment.
| Unlike calling an acceleration candidate, this does not resolve or freeze the | |
| implementation. Asking what is active never commits the domain to an answer, | |
| so a ``None`` result does not become stale merely by being observed. | |
| Unlike calling an acceleration candidate, this does not resolve or freeze the | |
| implementation. |
| with domain_._lock: | ||
| names = set(domain_._engine_registry) | ||
| # Engines advertised via entry points may not have been imported yet, in | ||
| # which case they are absent from the registry above but are still valid | ||
| # arguments to enable_engine(), which loads the plugin on demand. | ||
| names.update(ep.name for ep in entry_points(group=f"coheriq.engines.{domain_._name}")) |
There was a problem hiding this comment.
I feel like at some point this mechanism is likely to show its age, but I also think it is good enough for right now. For instance, a user might want some way of querying whether an engine has been imported yet, or the package that provides a given engine. (Is it safe to query entry_points once for an engine and then cache the results? If so, then another function could query this extra info by name, without breaking the return value here.)
| # which case they are absent from the registry above but are still valid | ||
| # arguments to enable_engine(), which loads the plugin on demand. | ||
| names.update(ep.name for ep in entry_points(group=f"coheriq.engines.{domain_._name}")) | ||
| return tuple(sorted(names)) |
There was a problem hiding this comment.
I suspect at some point in the future, there will be a way to optionally provide a priority value with an engine, so that something other than the reference engine can be activated by default if it is available. I do feel though that only a domain itself should be able to provide an engine that will displace its reference implementation by default. A third-party engine should not be able to activate itself automatically just because it is installed.
So, sorted is fine for now, but probably eventually the higher priority ones should be first.
But this also suggests that we could use "reference" as a placeholder; in that case, the references with positive priority could be listed by highest-to-lowest priority, then "reference", then those in descending order of priority. When priority is tied, then those are alphabetized.
If we were to add "reference" to this, then I feel it would also be appropriate to rename the companion function to active_engine.
| ``None`` therefore means "not decided yet", and never "decided on no engine"; | ||
| the latter is reported as :data:`~coheriq.REFERENCE`. Because |
There was a problem hiding this comment.
| ``None`` therefore means "not decided yet", and never "decided on no engine"; | |
| the latter is reported as :data:`~coheriq.REFERENCE`. Because | |
| ``None`` therefore means "not decided yet", while :data:`~coheriq.REFERENCE` means "decided on no engine". | |
| Because |
|
This will probably be closed in favor of #25. |
Adds the two user-facing functions anticipated when
enable_enginebecame a free function. The argument for that shape was that a domain author shouldn't have to re-export each new user-facing function as the API grows, so these follow it rather than becoming domain methods.They're in one PR because they're coupled:
available_engineshas to solve entry-point discovery, and what it chooses to report is whatactive_implementationis meaningfully compared against. Splitting them would mean writing the discovery logic once and immediately reopening it.Reviewable as two commits. The first adds both functions; the second revises the return protocol of the second function, described below.
available_engines(domain) -> tuple[str, ...]Returns the sorted names of engines that can be enabled. This unions two sources, because
enable_enginealready accepts either:coheriq.engines.<domain>entry point group, which may not have been imported yetAdvertised plugins are deliberately not imported in order to answer the question — an introspection call shouldn't execute arbitrary third-party import code as a side effect. The consequence is stated in the docstring: a listed name means
enable_enginewill attempt it, not that it's guaranteed to load.active_implementation(domain) -> str | NoneReturns which implementation the domain has resolved to. Three possible results:
NoneREFERENCE("reference")The domain has two distinct no-engine states: not yet resolved, and already frozen on its own implementation. From the caller's side those are opposite answers — in the first an engine can still be enabled, in the second it is permanently too late. Returning
Nonefor both would collapse the actionable state into the settled one, soNoneis reserved for "not decided yet" and the frozen case is reported asREFERENCE.This is worth the tri-state because
MATERIALIZEDisn't a transient startup instant. It's a stable, long-lived state, and the only one in whichenable_enginestill works — so "can I still act?" is the question a caller most plausibly has, and it should be answerable without a second call.Because
"reference"is a possible result, it can't also be an engine name.AccelerationEngine.__init__rejects it, via a_RESERVED_ENGINE_NAMESdenylist that sits alongside the existing non-string and empty-name validation and raises the sameCoheriqEngineError. Adding to that denylist later would break any engine already using the name, so it's kept deliberately small and the comment says so.available_engines()reports engines only and never includes"reference".Why "reference"
It's the ordinary HPC sense — reference BLAS, reference LAPACK: the straightforward implementation that defines correct behavior and that accelerated ones are checked against. That framing is also what justifies the function's name. The domain's own implementation is one of the possible answers, so
active_enginewas too narrow a word for what's being returned;active_implementationcovers the whole range.The asymmetry with
available_enginesis intentional. The two functions report genuinely different categories of thing — one enumerates engines, the other names whichever implementation is in effect, engine or not.Neither function resolves the implementation
Asking what is active must not commit the domain to an answer — otherwise a read-only-looking call would silently foreclose enabling an engine afterwards. There's no reason to force a user onto the reference implementation merely because they asked a question about the current state. Both functions have an explicit test for this, and
active_implementationadditionally has one asserting that repeated calls don't freeze.This is also what keeps the tri-state honest: a
Noneresult doesn't become stale merely by being observed.Why a sorted tuple and not a set
Engine names are conceptually an unordered set, so
setorfrozensetwould be defensible. Sorted tuple won on two practical grounds:--helptext, "try one of: ..." error messages.setiteration order for strings varies between processes, which would make all of those unstable, and this repo runs doctests as a tox environment.setis mutable and looks like it might be live; someone could.add()to it and expect that to register an engine. A tuple reads as the snapshot it is (the registry is copied under the domain lock, then the lock is released).Callers wanting set algebra can wrap in
set(...).Implementation note
active_implementationclassifies the domain states explicitly rather than treating "notENGINE_ENABLED" as the undecided case. If a state is added later, that forces a deliberate decision about how it's reported instead of silently defaulting toNone.The domain-or-name coercion in
enable_engineis factored out into a shared_resolve_domainhelper, now that three public functions take that same argument.Testing
tox -e py— 50 passed, of which 26 are newtox -e lint— green: ruff, copyright check, mypy, pylint 10.00/10 onsrc/andtest/, nbqa, renotox -e doctest,tox -e docs— both greenavailable_enginestests cover: the empty case; sorting; accepting a domain name as well as an object; unknown domain names; engines reportable before materialization; entry-point names appearing; an entry point not duplicating an already-registered engine; other domains' entry points being ignored; and not resolving the implementation. Entry-point discovery is tested by monkeypatchingentry_pointsin theactivationmodule, since the repo had no existing pattern for exercising that path.active_implementationtests cover all three results and the boundaries between them:Nonebefore materialization and before resolution;REFERENCEafter freezing on the reference implementation, including via an empty env var;REFERENCEbeing distinct fromNoneacross the transition; the enabled engine's name, including among several engines and via environment-variable activation; not resolving the implementation; and repeated calls not freezing.Reserved names are covered separately:
"reference"rejected as an engine name both as a literal and via the constant, the check being exact rather than case-insensitive, andREFERENCEnever appearing inavailable_engines().Notes
REFERENCEconstant are added todocs/apidocs/user_functions.rst.releasenotes/notes/, so it sets the file-layout precedent.reno.sphinxextis loaded indocs/conf.py, but no page uses therelease-notesdirective, so notes don't appear in the built docs yet. Not addressed here.This PR was generated by Claude Opus 5 under my guidance.