Skip to content

Commit 946fd4f

Browse files
authored
Fix nnUNet runner store_true flag command construction (#8941) (#8944)
### Description `nnUNetV2Runner.train_single_model_command` builds the `nnUNetv2_train` argv by iterating `kwargs` and always appending `str(_value)`, so documented `store_true` flags were emitted with a value instead of bare. Passing `c=True` produced `--c True`, `val=True` produced `--val True`, and likewise for `use_compressed` and `disable_checkpointing`. `nnUNetv2_train` declares these as `store_true`, so the trailing `True` is parsed as a positional argument and the command fails. A falsy `pretrained_weights` was similarly emitted as `-pretrained_weights False` instead of being omitted. The builder now appends `store_true` flags only when their value is truthy and skips them otherwise, and includes `pretrained_weights`/`-p` only when given a real path. Regular value kwargs and the existing `--npz` handling are unchanged. Fixes #8941, originally flagged in a review thread on #8887. ### Types of changes - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] New tests added to cover the changes. - [ ] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [ ] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [ ] In-line docstrings updated. --------- Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
1 parent 482d1d9 commit 946fd4f

3 files changed

Lines changed: 98 additions & 3 deletions

File tree

monai/apps/nnunet/nnunetv2_runner.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -596,9 +596,16 @@ def train_single_model_command(
596596
if self.export_validation_probabilities:
597597
cmd.append("--npz")
598598

599+
store_true_flags = {"c", "val", "use_compressed", "disable_checkpointing"}
599600
for _key, _value in kwargs.items():
600-
prefix = "-" if _key in {"p", "pretrained_weights"} else "--"
601-
cmd += [f"{prefix}{_key}", str(_value)]
601+
if _key in store_true_flags:
602+
if _value:
603+
cmd.append(f"--{_key}")
604+
elif _key in {"p", "pretrained_weights"}:
605+
if _value:
606+
cmd += [f"-{_key}", str(_value)]
607+
else:
608+
cmd += [f"--{_key}", str(_value)]
602609

603610
cmd_str: list[str] = [str(c) for c in cmd]
604611

@@ -758,7 +765,7 @@ def validate_single_model(self, config: str, fold: int, **kwargs: Any) -> None:
758765
kwargs: this optional parameter allows you to specify additional arguments defined in the
759766
``train_single_model`` method.
760767
"""
761-
self.train_single_model(config=config, fold=fold, only_run_validation=True, **kwargs)
768+
self.train_single_model(config=config, fold=fold, val=True, **kwargs)
762769

763770
def validate(
764771
self, configs: tuple = (M.N_3D_FULLRES, M.N_2D, M.N_3D_LOWRES, M.N_3D_CASCADE_FULLRES), **kwargs: Any

tests/apps/nnunet/__init__.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Copyright (c) MONAI Consortium
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Copyright (c) MONAI Consortium
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
12+
from __future__ import annotations
13+
14+
import unittest
15+
from unittest import mock
16+
17+
from monai.apps.nnunet.nnunetv2_runner import nnUNetV2Runner
18+
19+
20+
def _make_runner(export_validation_probabilities=False):
21+
runner = nnUNetV2Runner.__new__(nnUNetV2Runner)
22+
runner.dataset_name_or_id = "001"
23+
runner.trainer_class_name = "nnUNetTrainer"
24+
runner.export_validation_probabilities = export_validation_probabilities
25+
return runner
26+
27+
28+
class TestTrainSingleModelCommand(unittest.TestCase):
29+
def test_store_true_flags_emit_bare(self):
30+
runner = _make_runner()
31+
cmd, _ = runner.train_single_model_command(
32+
"3d_fullres", 0, 0, {"c": True, "val": True, "use_compressed": True, "disable_checkpointing": True}
33+
)
34+
for flag in ("--c", "--val", "--use_compressed", "--disable_checkpointing"):
35+
self.assertIn(flag, cmd)
36+
self.assertNotIn("True", cmd)
37+
38+
def test_store_true_flags_false_omitted(self):
39+
runner = _make_runner()
40+
cmd, _ = runner.train_single_model_command(
41+
"3d_fullres", 0, 0, {"c": False, "val": False, "use_compressed": False, "disable_checkpointing": False}
42+
)
43+
for flag in ("--c", "--val", "--use_compressed", "--disable_checkpointing"):
44+
self.assertNotIn(flag, cmd)
45+
self.assertNotIn("False", cmd)
46+
47+
def test_pretrained_weights_truthy_included(self):
48+
runner = _make_runner()
49+
cmd, _ = runner.train_single_model_command("3d_fullres", 0, 0, {"pretrained_weights": "/path/to/weights.pth"})
50+
self.assertIn("-pretrained_weights", cmd)
51+
self.assertIn("/path/to/weights.pth", cmd)
52+
53+
def test_pretrained_weights_falsy_omitted(self):
54+
runner = _make_runner()
55+
cmd, _ = runner.train_single_model_command("3d_fullres", 0, 0, {"pretrained_weights": False})
56+
self.assertNotIn("-pretrained_weights", cmd)
57+
self.assertNotIn("False", cmd)
58+
59+
def test_value_kwargs_unaffected(self):
60+
runner = _make_runner()
61+
cmd, _ = runner.train_single_model_command("3d_fullres", 0, 0, {"npz": "something"})
62+
self.assertIn("--npz", cmd)
63+
self.assertIn("something", cmd)
64+
65+
66+
class TestValidateSingleModelCommand(unittest.TestCase):
67+
def test_validate_emits_bare_val_flag(self):
68+
runner = _make_runner()
69+
with mock.patch("monai.apps.nnunet.nnunetv2_runner.run_cmd") as run_cmd:
70+
runner.validate_single_model("3d_fullres", 0)
71+
cmd = run_cmd.call_args.args[0]
72+
self.assertIn("--val", cmd)
73+
self.assertNotIn("--only_run_validation", cmd)
74+
self.assertNotIn("True", cmd)
75+
76+
77+
if __name__ == "__main__":
78+
unittest.main()

0 commit comments

Comments
 (0)