WIP 539 Clarify purpose of run_3dcnn_training_in_swarm in ODELIA CI

[oleschwen]

• Clarified that CI includes test to run one non-minimal 3D CNN training in simulated swarm
• Refactored integration test script to consistently use ResNet18 in tests using non-minimal 3DCNN
• WIP Extended manual/weekly test to include …

[Ultimate-Storm]

I looked into the failing unit test here, and the short version is: your change is right and the test is asserting a fiction. But there is a real bug sitting next to it that I think this PR should take with it.

The failing test is encoding behaviour that never worked

tests/unit_tests/test_env_config.py::TestLoadEnvironmentVariables::test_defaults_when_optional_vars_missing
    assert result["model_name"] == "ResNet101"  # default
E   AssertionError: assert None == 'ResNet101'

ResNet101 is not a registered model. CHALLENGE_MODELS contains exactly:

['1DivideAndConquer', '2BCN_AIM', '3agaldran', '4LME_ABMIL', '5Pimed']

So on main today, with MODEL_NAME unset, the resolution chain is:

1. env_config → model_name = 'ResNet101'
2. get_unified_model_name → 'ResNet101' is not "challenge" and not in get_all_model_names(), so it falls through to model_name = 'ResNet101'
3. get_model_config(logger, 'ResNet101') → not found → logger.info("No Configuration for model ResNet101 found. Use default model <MST>") → returns MST

The documented ResNet101 default has always silently trained MST. Your change to default=None produces the same MST, so it is not a behaviour regression — it just stops the code claiming a default that never applied. The test should be updated to assert None.

The bug worth taking with it

Removing the misleading string doesn't remove the silent fallback, it just makes it the only one left. get_model_config answers an unrecognised model name by logging at INFO and quietly substituting MST. With your change the operator now gets:

Use model variant as model name: None
No Configuration for model None found. Use default model <MST>

— two INFO lines, and a run that trains a model nobody asked for. That is precisely #541 ("Swarm runs do not verify that all sites resolved the same MODEL_NAME"): one site with a typo'd or unset MODEL_NAME silently trains MST while the other seven train the real model, and the run still reports success.

Since this PR is what makes --model_name the explicit way to choose a model, it is the natural place to make an unresolvable name fail loudly rather than fall back:

def get_model_config(logger, model_name: str):
    if model_name in get_all_model_names():
        return CHALLENGE_MODELS.get(model_name)
    if model_name in DEFAULT_MODEL:
        return DEFAULT_MODEL[model_name]
    raise ValueError(
        f"Unknown model {model_name!r}. Set MODEL_NAME or pass --model_name. "
        f"Available: {sorted(get_all_model_names()) + sorted(DEFAULT_MODEL)}"
    )

Note this also makes MST selectable by name, which it currently is not — it can only be reached by getting the name wrong.

One piece of dead code while you are in here

model_variant = env_vars.get('model_name', 'MST')

The 'MST' default can never fire. env_config builds its dict as a literal, so 'model_name' is always a key — previously with value 'ResNet101', now with value None. {'model_name': None}.get('model_name', 'MST') returns None, not 'MST'. Worth deleting or turning into an explicit if ... is None so it doesn't read as a working safety net.

Suggested shape

1. Update test_defaults_when_optional_vars_missing to assert None — it is codifying a default that resolved to MST anyway.
2. Raise on an unresolvable model name instead of falling back to MST, and make MST selectable by name.
3. Drop the unreachable 'MST' default in get_unified_model_name.
4. Add a test that an unknown MODEL_NAME raises rather than silently training something else.

Happy to push 1–4 onto this branch if you'd rather not — say the word. The CI-side change (--job ODELIA_ternary_classification --model_name $MODEL_NAME) looks right to me and is a clear improvement over smuggling the model through the job name.

[oleschwen]

Good points that should be addressed in a separate issue.
I'd say the purpose (maybe I didn't describe this well) of this issue/PR is to clarify what the run_3dcnn_training_in_swarm test is about and implement what is intended: checking whether either one "productive" (i.e., not just the minimal example) model can be trained for more than one round in a simulated swarm (in which case it's fine to explicitly specify said model), or use the current default model for that purpose. I would go for the former and consider adding a swarm test of all available models as part of the weekly/manually triggered test.
I fully agree that specifying a ResNet variant must not result in an MST being trained. We should consider how to make defaults at different locations consistent or whether we want to drop default behavior (to avoid silently doing something potentially unintended), e.g., as part of #546.