Fix live reduction when Mantid default facility is not SNS

[darshdinger]

Description of work

Live reduction fails outright for any user whose Mantid default facility is not SNS. The reported
failure came through SNAPWrap on an analysis node:

Invalid value for property Instrument (string) from string "SNAP": When setting value of
property "Instrument": The value "SNAP" is not in the list of allowed values
snapred.backend.error.StateValidationException: Instrument State for given Run Number is invalid!

Mantid's live-data algorithms (LoadLiveData, StartLiveData, MonitorLiveData) build the
allowed-values list for their Instrument property from the default facility, restricted to
instruments that have a live-data listener. Under a non-SNS default that list comes back empty, so
Instrument="SNAP" is rejected before anything else happens. SNAPRed has no argument it can pass to
avoid this.

This is a Mantid defect, and after review the agreed direction is to fix it upstream rather than work
around it in SNAPRed. So this PR does not itself make live reduction work under a non-SNS default —
see "What this PR does not do" at the bottom. What it does is remove SNAPRed's own dependence on
Mantid's process-wide defaults, and record the defect and its workaround so the next person doesn't
have to rediscover any of it.

Explanation of work

Two rules came out of review, and they now apply to all SNAPRed code:

1. Never depend on the Mantid default facility or default instrument. Resolve both by name —
   getFacility(<name>) and getInstrument(<name>) search all facilities and are unaffected by the
   user's defaults; the no-argument forms are not.
2. Wherever an algorithm accepts an instrument, it should also accept a facility. An instrument
   name is only meaningful with respect to a facility. Either may be optional, but both must be
   overridable.

Changes follow from those:

LoadLiveDataInterval now declares a Facility property alongside Instrument, and its
validateInputs resolves against a named facility instead of the default one. An empty Facility
means <liveData.facility.name> — deliberately not Mantid's default facility:

instrumentName = self.getProperty("Instrument").value
facilityName = self.getProperty("Facility").value or Config["liveData.facility.name"]
facility = ConfigService.getFacility(facilityName)   # by name, never getFacility()
facility.instrument(instrumentName)

Worth flagging one trap found while writing this: the two lookups report different sentinels. An
unknown facility raises Facilities search object, while an unknown instrument within a facility
raises FacilityInfo search object. My first version guarded on the wrong one and re-raised instead of
returning a clean validation error; a test caught it.

Facility cannot yet be forwarded to the child LoadLiveData — Mantid has no such property — so
there's a TODO at that call site pointing at the upstream fix.

CheckIPTS had two problems. The "DAS" exclusion compared an InstrumentInfo against a string,
so it silently did nothing; "DAS" matters precisely because it is what ConfigService.setFacility("SNS")
selects as the default instrument. Separately, it logged "Using default instrument: X" while actually
passing the empty instrument to FileFinder — a misleading message about a default-instrument
dependence. Both fixed.

SNAPRedGUI now saves and restores default.instrument alongside default.facility. Setting the
facility alone never yields the right instrument: setString leaves the previous one in place, while
ConfigService.setFacility resets it to the facility's first instrument. This is also, in effect,
the workaround applied automatically for GUI users.

Approach considered and withdrawn

An earlier revision of this branch scoped a temporary override of the default facility/instrument
around each live-data call, applied centrally in MantidSnapper.executeAlgorithm. It worked, but it
was withdrawn in review as too much complexity to carry for a defect in code we don't own — and it
required mutating a process-wide singleton the workbench reads concurrently, which brings its own
hazards. That mechanism, and the MantidSnapper changes it needed, have been reverted in full;
MantidSnapper.py is byte-identical to next apart from one added comment. It remains in this
branch's history at e0e4ef5e if it is ever wanted.

That approach also needed the configuration correct at construction time, so it moved algorithm
construction inside the live-data mutex — justified in review comments by an appeal to listener
safety, which was wrong: constructing the algorithm creates no listener. The mutex guards execution
deliberately, because LoadLiveData has been used as a stay-resident algorithm whose instance is kept
alive across execute calls so its listener can preload and keep working against the same stream.
That rationale wasn't recorded anywhere and was nearly lost, so it is now captured both as a comment
at the mutex declaration and in the implementation note. The comment is the only change to
MantidSnapper.py; drop it if you'd rather this PR left that file completely alone.

Three facts about the Mantid validator are recorded in the new implementation note, because they rule
out the obvious upstream shortcuts:

• The allowed-values list is fixed at initialize() and never re-evaluated. AlgorithmManager.create
  initializes; a second initialize() is a no-op.
• Therefore adding a Facility property upstream is not sufficient on its own — property values
  are only set after initialization. The validation must also be made dynamic, e.g. by moving the
  check into validateInputs.
• Setting Listener/Address explicitly does not avoid the lookup. With Instrument empty, the
  algorithm resolves from the default instrument regardless — verified for Listener alone,
  Address alone, both, and neither, all identical.

To test

Dev testing

• tests/unit/backend/recipe/algorithm/test_LoadLiveDataInterval.py — 52 tests, of which 4 are new
  (TestLoadLiveDataIntervalFacility): SNAP validates while the default facility is ILL; an explicit
  Facility is honoured; an instrument outside the named facility is rejected; an unknown facility is
  rejected. Existing tests updated for the new property and the changed error text.
• tests/unit/backend/data/test_liveDataFacilityConfig.py — 9 tests characterizing the Mantid
  behaviour: the rejection itself, that facility+instrument together fix it, that amend_config
  restores cleanly, that allowed-values are frozen at initialize(), and that getInstrument(name)
  searches all facilities. These assert nothing about SNAPRed, so they stay valid before and after the
  upstream fix — when it lands, the "rejected" expectations should start failing, which is the signal
  this workaround can be retired.
• tests/unit/backend/recipe/algorithm/test_CheckIPTS.py — 5 tests: DAS excluded, SNAP present,
  empty string still allowed.
• tests/unit/ui/test_mainLiveDataConfig.py — 4 tests for the GUI save/override/restore, run
  against a stub so no Qt widgets are constructed.

Things reviewers should look at closely:

• tests/unit/ui/test_mainLiveDataConfig.py is importorskip-guarded, because snapred.ui.main
  requires the pinned Mantid version and my local env is behind. It skips locally and runs in CI. I
  verified the logic separately by shimming the one missing symbol, so I expect it to pass rather than
  merely skip — but CI is the first place it actually executes.
• My local .pixi/envs/dev is on Mantid 6.15 against the >=6.16 pin, because pixi cannot read
  lockfile v7 without an upgrade. All of tests/unit/ui therefore fails to collect locally, so the
  suite was run as pytest tests/unit --ignore=tests/unit/ui: 1517 passed, 31 skipped, 1 failed.
  The one failure (test_RunMetadata::test_defaults_log_warning_fromNeXusLogs) is pre-existing —
  confirmed by stashing on a clean tree.

CIS testing

Run from workbench. This checks that SNAPRed's own validation no longer depends on your default
facility, and that the documented workaround does what the note says.

#
# EWM 15513: SNAPRed's own live-data validation must not depend on the Mantid default facility.
#
from mantid.simpleapi import *
from mantid.kernel import ConfigService, amend_config

from snapred.backend.recipe.algorithm.LoadLiveDataInterval import LoadLiveDataInterval
from snapred.meta.Config import Config

FACILITY_KEY, INSTRUMENT_KEY = "default.facility", "default.instrument"
config = ConfigService.Instance()
saved = (config[FACILITY_KEY], config[INSTRUMENT_KEY])
print(f"your Mantid defaults: {saved[0]} / {saved[1]}")

try:
    # Deliberately set a default facility that is NOT SNS.
    config.setString(FACILITY_KEY, "ILL")
    config.setString(INSTRUMENT_KEY, "IN5")
    print(f"forced defaults to: {config[FACILITY_KEY]} / {config[INSTRUMENT_KEY]}")

    # 1. SNAPRed's own algorithm must still validate SNAP.  EXPECTED: no 'Instrument'/'Facility' error.
    algo = LoadLiveDataInterval()
    algo.initialize()
    algo.setProperty("OutputWorkspace", "cis_probe_ws")
    algo.setProperty("Instrument", Config["instrument.name"])
    errors = algo.validateInputs()
    assert "Instrument" not in errors and "Facility" not in errors, f"UNEXPECTED: {errors}"
    print(f"PASS: LoadLiveDataInterval validates SNAP under an ILL default (errors={errors})")

    # 2. Mantid's own LoadLiveData still rejects SNAP -- this is the upstream defect, not yet fixed.
    mantidAlgo = AlgorithmFactory.create("LoadLiveData", -1)
    mantidAlgo.initialize()
    allowed = list(mantidAlgo.getProperty("Instrument").allowedValues)
    print(f"EXPECTED-FAILURE: Mantid LoadLiveData allowed instruments under ILL = {allowed}")
    try:
        mantidAlgo.setProperty("Instrument", Config["instrument.name"])
        print("NOTE: Mantid accepted SNAP -- the upstream fix may have landed; see the impl note.")
    except ValueError as e:
        print(f"EXPECTED-FAILURE confirmed: {e}")

    # 3. The documented workaround must make it work.
    with amend_config(facility=Config["liveData.facility.name"], instrument=Config["liveData.instrument.name"]):
        workaroundAlgo = AlgorithmFactory.create("LoadLiveData", -1)
        workaroundAlgo.initialize()
        workaroundAlgo.setProperty("Instrument", Config["instrument.name"])
        print("PASS: with the facility set, Mantid accepts SNAP")

    # 4. The workaround must not leak.
    assert config[FACILITY_KEY] == "ILL", f"facility leaked: {config[FACILITY_KEY]}"
    print("PASS: amend_config restored the previous facility")

finally:
    config.setString(FACILITY_KEY, saved[0])
    config.setString(INSTRUMENT_KEY, saved[1])
    print(f"restored your Mantid defaults: {config[FACILITY_KEY]} / {config[INSTRUMENT_KEY]}")

Success looks like: PASS: on steps 1, 3 and 4, and EXPECTED-FAILURE confirmed on step 2. Step 2
failing is the point — it is the upstream defect this PR documents rather than fixes.

GUI check. Set a non-SNAP default instrument in File → Settings → General, restart workbench,
open SNAPRed, then confirm in settings afterwards that your default instrument is unchanged — SNAPRed
must save and restore it, not leave its own behind.

Link to EWM item

EWM#15513

Verification

• the author has read the EWM story and acceptance critera
• the reviewer has read the EWM story and acceptance criteria
• the reviewer certifies the acceptance criteria below reflect the criteria in EWM

Acceptance Criteria

This list is for ease of reference, and does not replace reading the EWM story as part of the review.
Verify this list matches the EWM story before reviewing.

These need confirming against the story, and possibly renegotiating. The story's stated
expectation is that SNAPRed reduces SNAP data irrespective of the default instrument setting. This
PR does not achieve that on its own — it requires the user to set their facility until the upstream
Mantid fix lands. That is a deliberate decision taken in review, but it needs the story owner's
agreement before this can close.

• SNAPRed's own code never depends on the Mantid default facility or default instrument
• SNAPRed does not permanently alter the user's Mantid default facility or instrument
• the Mantid defect and its interim workaround are documented for users and developers

What this PR does not do

• It does not make live reduction work under a non-SNS default facility. A user in that situation
  must still set their facility — via workbench settings, Mantid.user.properties, or
  ConfigService.setFacility("SNS"). The implementation note gives the instructions.
• The upstream Mantid fix is a separate PR. Make the live-data Instrument validation dynamic and
  add an optional Facility property. A prototype validates correctly under a non-SNS default with no
  configuration mutation at all, and gives far better diagnostics. Note it must not change
  LoadLiveData itself in ways that disturb the listener lifecycle.
• EWM#17188 is still open — why run 69405's PV file was not found. That absence is what routed the
  code into the live-data fallback to begin with. Malcolm has since confirmed the file exists at
  /SNS/SNAP/IPTS-36879/nexus/SNAP_69405.nxs.h5 with all PVs present, which sharpens rather than
  answers the question: it is worth checking on analysis whether the IPTS lookup is also
  facility-dependent, since CheckIPTS/FileFinder sits on that path.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.51%. Comparing base (c94cf60) to head (1794914).

Additional details and impacted files

@@           Coverage Diff           @@
##             next     #685   +/-   ##
=======================================
  Coverage   96.51%   96.51%           
=======================================
  Files          79       79           
  Lines        7279     7279           
=======================================
  Hits         7025     7025           
  Misses        254      254           

Flag	Coverage Δ
integration	48.77% <ø> (ø)
unittests	96.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ekapadi]

"""Naming the listener explicitly doesn't help. Setting Listener/Address with Instrument
empty still resolves from the default instrument, failing with "Attempted to access live listener
for instrument, which has no listeners."
"""
Did you mean "setting address with instrument empty"? Otherwise I'm not understanding what you mean here! :)

[ekapadi]

""" One side effect worth noting for review: algorithm creation moved from before the mutex to inside
it. That closes a window in which two threads could each construct a LoadLiveData — and so risk a
second listener — before either acquired the lock. """ -- I'm not sure about this change. Previously the lock protected execution, not constructing listeners (basically: those are just socket connections). So we need to be very careful about this!

[ekapadi]

I'm very skeptical about these changes! Arguably you've selected an optimal path, but I think we're adding lots of complexity, and future potential defects to work around Mantid flaws. We should probably just fix the latter. (And any users of SNAPwrap, in the mean time [i.e. waiting for the new Mantid version] can just set their facility!)

Here are what I think are the key points:

• No consumer algorithm in SNAPRed should be using the default instrument; In my own code changes, I've always tried to pass the instrument as an arg;
• Any algorithm that accepts an instrument arg should also be accepting a facility arg -- these two things just go together; either one might be optional, but it should be possible to override both;
• We had discussed the issue of changing the validation in [those specific Mantid algorithms] so that it is dynamic -- i.e. then this could be solved by just adding an optional facility arg to the live-data algorithms;
• I don't like putting anything into MantidSnapper. I also am "suspicious" about changes to the mutex -- it might be OK, but ...