-
Notifications
You must be signed in to change notification settings - Fork 4
Simplify memory_gpu field in method database entry #428
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
Simplify memory_gpu field in method database entry #428
Conversation
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.
Looks good to me, just a few comments 🙂 I'd like the comment about the code comment addressed, but the other one isn't such a big deal, feel free to disregard.
Bigger picture, the test locally are mostly passing but with some failures, due to the axis
parameter stuff that existed. This issue has recently been fixed, so can you update this branch with the changes from main
please; that should pick up:
- fixes to IRIS tests CI, so then the IRIS tests run ok
- fixes to docs CI
- updates to fix failing tests due to
axis
issues
…y_gpu entry structure
…tive to allow kwarg memory_gpu to be used in test_can_determine_max_slices_no_gpu_estimator inside test_task_runner
… GpuMemoryRequirement
7dda813
to
6d208c2
Compare
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.
Cool, thanks for this! 🙂
IRIS tests CI isn't happy only due to paganin memory hook tests, which is planned to get fixed soon in another PR, so this looks good to merge.
Weird, for some reason the task checker CI job is failing, even though all checkboxes have been ticked... I'm gonna consider that a separate issue, and will potentially look into later 😅 |
Fixes #356, #357, #358
Checklist