Fix/change default gpu type to a100#446
Conversation
There was a problem hiding this comment.
Pull request overview
This PR changes the default GPU type from "L4" to "A100" in the Aignostics Python SDK. The change updates the DEFAULT_GPU_TYPE constant in the platform module and ensures existing tests continue to work correctly by explicitly specifying "L4" where the previous default behavior was relied upon.
Changes:
- Updated default GPU type constant from "L4" to "A100"
- Modified tests to explicitly specify GPU type "L4" to maintain existing test behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/aignostics/platform/_constants.py | Changed DEFAULT_GPU_TYPE from "L4" to "A100" |
| tests/aignostics/application/cli_test.py | Added explicit --gpu-type L4 flags to 7 test cases to maintain test behavior with original GPU type |
| tests/aignostics/application/gui_test.py | Added explicit --gpu-type L4 flag to test case to maintain test behavior with original GPU type |
Comments suppressed due to low confidence (2)
src/aignostics/platform/_constants.py:40
- The CLI_REFERENCE.md documentation file still indicates the default GPU type is "L4" (lines 278 and 374), but this change updates the default to "A100". The documentation needs to be regenerated or updated to reflect this change.
DEFAULT_GPU_TYPE = "A100"
tests/aignostics/application/cli_test.py:160
- The tests in cli_pipeline_validation_test.py that validate invalid configurations (test_cli_run_submit_fails_on_invalid_gpu_provisioning_mode, test_cli_run_submit_fails_on_invalid_cpu_provisioning_mode, test_cli_run_submit_fails_on_max_gpus_per_slide_zero, test_cli_run_submit_fails_on_max_gpus_per_slide_too_high) do not explicitly specify a GPU type. This means they will now use A100 as the default instead of L4. For consistency with the other test updates in this PR and to ensure tests remain deterministic regardless of default changes, these tests should also explicitly specify the GPU type they want to use.
result = runner.invoke(
cli, ["application", "run", "submit", HETA_APPLICATION_ID, str(metadata_csv), "--gpu-type", "L4", "--force"]
)
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (62.72%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage.
|
8e6e749 to
5843102
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/aignostics/application/cli_test.py:1472
- This test hard-codes the GPU type as "L4" even though the file now imports
PIPELINE_GPU_TYPEfor other submissions. Using the shared constant here as well keeps the test aligned with the intended "keep L4 for tests" configuration and avoids future drift if the test GPU type changes again.
"Testing JSON format output - run 2",
"--gpu-type",
"L4",
],
5843102 to
4756a72
Compare
|
Change default GPU type for A100, keep L4 for tests