Test multi-endpoint, Default to Learning engine, Sunset StreamingActiveLearner

[BenCarter44]

The test/09-service is changed now to test the DT against two endpoints: inference and learning.

The agent and model investigator themselves carry the respective tag. This is a base-line example to test multiple endpoint runs.

Note: This PR switches the default backend to the learning backend. This is so that by default, all tasks run on the powerful "learning backend" rather than the remote inference machine somewhere else. Tests have been updated to reflect this.

Secondly, the number of inference tasks is much less than the number of learning tasks when using active learning. The user specifically selects the tasks for inference, but the remaining tasks deal with learning.

Also: this supersedes the "StreamingActiveLearner" logic, as now the runtime simply passes the backend name that the user declared.

[andre-merzky]

I'm afraid I mostly disagree with the PR. One part is fine: Multi-endpoint baseline test are useful (but some mechanical fixes are needed).

But the main part (in volume), the switch of roles, seems strange. Yes, there are fewer inference tasks - but labeling tasks is not difficult by volume, it's just code. More importantly, the default applies to latency-sensitive tasks most (sensors, sinks, selectors, plumbing in general), and the learning pool is most likely a remote HPC resource. It would be much cleaner to have a config option which decides to what pool unlabeled tasks go. Finally: did you measure the impact? This looks like an optimization - but I don't see this justified w/o measurements that this needs optimizing in the first place. What prompted that change, beyond 'fewer inference tasks'? That is a very weak justification for the large change.

Also, the switch is not completely applied - I attach my claude review in the next comment. But before acting on that, please respond to the above.

Final point, the streaming learner deprecation: that would warrant a separate PR (this PR really has three parts: example, pool switch, learner deprecation). Also I agree the routing part is superseded, but the learner does more than that, and IIUC, the framework is left w/o leaner implementation? I may be missing something here though...

[andre-merzky]

● Code review(review · 10 findings)
  ⎿  src/digitaltwin/service/session.py
       ● 511 [correctness]       The role-alias flip is one-way: a legacy config declaring only an 'inference' engine silently builds a phantom 'learning' backend from an empty config
                                 as the primary engine instead of aliasing to the configured role.
       ● 117 [correctness]       R8 endpoint-loss detection is broken for the inference role: twin.engines is seeded {ROLE_LEARNING} and the only code that added the inference role is
                                 commented out, so no twin ever records a binding to the inference engine.                                                                              
       ●  73 [correctness]       _is_learner is dead-but-armed: its only call site is commented out and the guarded import defining StreamingLearnerInvestigator was deleted, so the
                                 function references an undefined global and raises NameError if ever called.
     src/digitaltwin/service/__init__.py
       ●  21 [correctness]       ROLE_INFERENCE and ROLE_LEARNING were appended to __all__ unquoted, so __all__ contains the constant values 'inference'/'learning' instead of the
                                 exported names.
     src/digitaltwin/learn.py
       ● 138 [correctness]       The StreamingActiveLearner import is commented out but StreamingLearnerInvestigator.__init__ still calls it, so the still-shipped, still-documented
                                 class imports cleanly and crashes with NameError at instantiation even with ROSE installed.
     test/integration/conftest.py
       ● 284 [efficiency]        RADICAL_ORBIT_RHAPSODY_NOTIFY_WINDOW='0' moved off the endpoint the tests actually execute on: learning_endpoint now runs the default 250 ms window
                                 while the tuning sits on inference_endpoint, which no integration test component uses.
     test/09-service/run_me.py
       ●  55 [correctness]       ENGINES hardcodes endpoints 'pi' (concurrent) and 'hpc' (dragon) while the README still instructs starting one endpoint named 'dt_task_ep' steered via
                                 DT_INFERENCE_ENDPOINT, which is now a silent no-op.
     src/digitaltwin/service/client.py
       ●  77 [correctness]       The role-contract flip missed every site outside session.py: client.py still documents 'learning aliases inference when not configured' (the inverse of
                                 the new behavior), session.py docstrings still call inference the base role, and the dashboard still renders the alias badge the old way.
     test/100-resource-aware-selector/profiler/all2.csv
       ●   1 [unrelated-content] ~3,400 lines of generated telemetry and scratch scripts unrelated to the PR are committed: five CSV dumps (two byte-identical), placeholder JSON, and a
                                 scratchwork/ dir whose test_profiler.py imports a module that exists nowhere on the branch.
     test/old/old_test_learn.py
       ●   7 [test-hygiene]      The learner sunset parks code half-alive instead of deleting it: a module-level exit() disables the relocated test file and ~150 further lines are
                                 commented out across four test files, leaving dual/doomed fixtures with no consumer.

● Review of PR #25 done — 10 confirmed findings, all independently verified against the branch. Not mergeable as-is.

  The findings cluster into three themes:

  1. The role-contract flip is half-applied. The alias now only works learning→inference wise; a legacy inference-only config silently builds a phantom auto-placed learning backend, R8
     endpoint-loss detection no longer fires for the inference role (the test was edited to mask it), and every docstring, the client, and the dashboard still describe the old direction.
  2. The learner sunset is half-done. StreamingLearnerInvestigator still ships and still crashes with NameError at instantiation, _is_learner references a deleted import, and ~150 lines are
     parked as comments or behind a bare exit() in test/old/ that aborts any pytest test/ run at collection.
  3. Hygiene: unquoted names in __all__, ~3,400 lines of unrelated scratch telemetry (one CSV committed twice byte-identical), the 09-service demo hardcoding nonexistent endpoints against
     its own README, and the notify-window tuning moved onto the endpoint the integration tests don't use (up to 250 ms per round trip of new CI latency).

  The two findings I'd hold the review on hardest are the phantom-backend alias bug and the broken R8 detection — both silent-failure modes on multi-day runs. Say the word if you want these
  posted as comments on the PR (individually inline, or as one summary comment).

[BenCarter44]

Hello Andre,

Thank you for the feedback. Yes, there are three things here, and I agree that three PRs would be cleaner.

---

First part: Multi-endpoint baseline test. That is the most important item currently.

---

Second part: Yes, I didn't remove every mention about StreamingActiveLearner as I wanted to talk with you first.

So, what I was originally thinking is that the DT framework itself doesn't need to have a streaming learner implementation at all. That is the point of the ModelInvestigator abstraction API. See test/rose_agent and test/rose_example. If a user wants to integrate streams with their learner, they define their own logic in a subclass of ModelInvestigator.

Inside the science agent and model investigators, there already is the interface for exsitu out-of-stream sections and insitu in-stream sections. So, user code simply extends this with their own implementation / backend labels.

BUT: I agree with you that a lot of what the user do will be repeated boilerplate. So, what if StreamingLearnerInvestigator was edited to the following:

• Remove the StreamingActiveLearner dependency. Have the user provide the learner they wish to use in the constructor.
• The StreamingLearnerInvestigator does its own simple batching.
• The StreamingLearnerInvestigator publishes models generated by the learner.

---

Third part: Switch of roles. I agree with you on the config option for switching the default.

The main reason behind the switch was that we know ahead-of-time the inference tasks. We don't know ahead-of-time all the workflows/tasks that will run for learning. This is less of an optimization, but more to fit incoming tasks better to the correct roles.

Currently, with inference as the default, any workflow or task ROSE creates will be sent to the inference backend. This is a problem, as these are learning tasks which take considerable resources.
From this then, there are two solutions: engineer wrappers that trap and edit ROSE's calls, or (in my opinion better and much simplier) idea is to default all to learning, and just label the few known-ahead-of-time inference tasks to the inference backend. This solution is simpler and better ties to the use cases of digital twins in my opinion.

---

In the meantime: I will split this into three PRs / Issues

[BenCarter44]

Alright: split into three:

1. Multi-endpoint baseline test: Multi-endpoint baseline #26
2. Rework StreamingLearnerInvestigator: Rework the StreamingLearnerInvestigator #28
3. Config option to select default engine: Config option to select default engine for task execution #27

[BenCarter44]

For point 2, see draft of new StreamingLearnerInvestigator at #29