-
Notifications
You must be signed in to change notification settings - Fork 16
113 kubernetes integration #130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
46fd341
e9eece3
a538d42
cead76b
39ccaff
573842e
e99a837
e63c443
ad42a55
a59b90e
90a16c5
cf340a2
f399c7e
a78638b
bef0219
3527cf1
a7c9761
27950c4
2948475
14d5083
09fa7a2
b1d42e4
eff0050
fa1e555
978ca74
0f5d10f
4143fe4
0a26019
c1eea9b
7c1e5df
8f457e7
20eae49
915a556
379dace
654bbff
bdd5288
23cc758
56215a3
b924c5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| from brats.core.docker import run_container as run_docker_container | ||
| from brats.core.singularity import run_container as run_singularity_container | ||
| from brats.core.kubernetes import run_job as run_kubernetes_job | ||
| from brats.utils.algorithm_config import load_algorithms | ||
| from brats.constants import OUTPUT_NAME_SCHEMA, Algorithms, Task, Backends | ||
| from brats.utils.data_handling import InferenceSetup | ||
|
|
@@ -163,6 +164,7 @@ def _get_backend_runner(self, backend: Backends) -> Optional[Callable]: | |
| backend_dispatch = { | ||
| Backends.DOCKER: run_docker_container, | ||
| Backends.SINGULARITY: run_singularity_container, | ||
| Backends.KUBERNETES: run_kubernetes_job, | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Any misspelled key (e.g.
VALID_KUBERNETES_KWARGS = frozenset({
"namespace", "pvc_name", "pvc_storage_size",
"pvc_storage_class", "job_name", "data_mount_path",
})
unknown = set(kubernetes_kwargs) - VALID_KUBERNETES_KWARGS
if unknown:
raise ValueError(f"Unknown kubernetes_kwargs keys: {unknown}") |
||
| runner = backend_dispatch.get(backend, None) | ||
| return runner | ||
|
|
@@ -173,6 +175,7 @@ def _infer_single( | |
| output_file: Path | str, | ||
| log_file: Optional[Path | str] = None, | ||
| backend: Backends = Backends.DOCKER, | ||
| kubernetes_kwargs: Optional[Dict] = None, | ||
| ) -> None: | ||
| """ | ||
| Perform a single inference run with the provided inputs and save the output in the specified file. | ||
|
|
@@ -181,7 +184,8 @@ def _infer_single( | |
| inputs (dict[str, Path | str]): Input Images for the task | ||
| output_file (Path | str): File to save the output | ||
| log_file (Optional[Path | str], optional): Log file with extra information. Defaults to None. | ||
| backend (Backends | str, optional): Backend to use for inference. Defaults to Backends.DOCKER. | ||
| backend (Backends, optional): Backend to use for inference. Defaults to Backends.DOCKER. | ||
| kubernetes_kwargs (Optional[Dict], optional): Optional keyword arguments for Kubernetes Backend. Defaults to None. | ||
| """ | ||
| with InferenceSetup(log_file=log_file) as (tmp_data_folder, tmp_output_folder): | ||
| logger.info(f"Performing single inference") | ||
|
|
@@ -199,13 +203,22 @@ def _infer_single( | |
| runner = self._get_backend_runner(backend) | ||
| if runner is None: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation order: the
Suggested reorder: if kubernetes_kwargs is not None and backend != Backends.KUBERNETES:
raise ValueError("kubernetes_kwargs can only be used with the Kubernetes backend.")
runner = self._get_backend_runner(backend)
if runner is None:
raise ValueError(f"Unsupported backend: {backend}")The same issue is present in |
||
| raise ValueError(f"Unsupported backend: {backend}") | ||
| runner( | ||
| runner_kwargs = dict( | ||
| algorithm=self.algorithm, | ||
| data_path=tmp_data_folder, | ||
| output_path=tmp_output_folder, | ||
| cuda_devices=self.cuda_devices, | ||
| force_cpu=self.force_cpu, | ||
| ) | ||
| if kubernetes_kwargs is not None: | ||
| logger.debug(f"Adding Kubernetes kwargs: {kubernetes_kwargs}") | ||
| if backend != Backends.KUBERNETES: | ||
| raise ValueError( | ||
| "Kubernetes kwargs can only be used with the Kubernetes backend." | ||
| ) | ||
| for key, value in kubernetes_kwargs.items(): | ||
| runner_kwargs[key] = value | ||
| runner(**runner_kwargs) | ||
| self._process_single_output( | ||
| tmp_output_folder=tmp_output_folder, | ||
| subject_id=subject_id, | ||
|
|
@@ -219,6 +232,7 @@ def _infer_batch( | |
| output_folder: Path | str, | ||
| log_file: Optional[Path | str] = None, | ||
| backend: Backends = Backends.DOCKER, | ||
| kubernetes_kwargs: Optional[Dict] = None, | ||
| ): | ||
| """Perform a batch inference run with the provided inputs and save the outputs in the specified folder. | ||
|
|
||
|
|
@@ -227,6 +241,7 @@ def _infer_batch( | |
| output_folder (Path | str): Folder to save the outputs | ||
| log_file (Optional[Path | str], optional): Log file with extra information. Defaults to None. | ||
| backend (Backends, optional): Backend to use for inference. Defaults to Backends.DOCKER. | ||
| kubernetes_kwargs (Optional[Dict], optional): Optional keyword arguments for Kubernetes Backend. Defaults to None. | ||
| """ | ||
| with InferenceSetup(log_file=log_file) as (tmp_data_folder, tmp_output_folder): | ||
|
|
||
|
|
@@ -246,14 +261,23 @@ def _infer_batch( | |
| if runner is None: | ||
| raise ValueError(f"Unsupported backend: {backend}") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicated The block that validates and merges Extract to a small helper: def _build_runner_kwargs(self, base_kwargs: dict, backend: Backends, kubernetes_kwargs: Optional[Dict]) -> dict:
if kubernetes_kwargs is not None:
if backend != Backends.KUBERNETES:
raise ValueError("kubernetes_kwargs can only be used with the Kubernetes backend.")
return {**base_kwargs, **kubernetes_kwargs}
return base_kwargs |
||
| # run inference in container | ||
| runner( | ||
| runner_kwargs = dict( | ||
| algorithm=self.algorithm, | ||
| data_path=tmp_data_folder, | ||
| output_path=tmp_output_folder, | ||
| cuda_devices=self.cuda_devices, | ||
| force_cpu=self.force_cpu, | ||
| internal_external_name_map=internal_external_name_map, | ||
| ) | ||
| if kubernetes_kwargs is not None: | ||
| logger.debug(f"Adding Kubernetes kwargs: {kubernetes_kwargs}") | ||
| if backend != Backends.KUBERNETES: | ||
| raise ValueError( | ||
| "Kubernetes kwargs can only be used with the Kubernetes backend." | ||
| ) | ||
| for key, value in kubernetes_kwargs.items(): | ||
| runner_kwargs[key] = value | ||
| runner(**runner_kwargs) | ||
|
|
||
| self._process_batch_output( | ||
| tmp_output_folder=tmp_output_folder, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Blocker] Top-level import causes
ImportErrorfor users without thekubernetespackageThis import runs unconditionally at module load time. Anyone who does
from brats.core.segmentation_algorithms import ...without havingkubernetesinstalled will get anImportError, even if they never use the Kubernetes backend.The import should be deferred to where it is actually needed:
This is consistent with how optional heavy dependencies are typically guarded and pairs with making
kubernetesan optional install extra inpyproject.toml.