Closed pedro-avalos closed 4 weeks ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 47.81%. Comparing base (
cd772da
) to head (ab38aaf
). Report is 17 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
One design question I'd like some feedback on is how rvs.py
takes the modules as positional arguments. Since we're likely to just use these one at a time in Checkbox jobs, should these be subparsers instead? If they are subparsers, then we could conceivably add the module configuration as options for those subparsers. The more I think about it, they should be subparsers. Let me know what you think!
akes the modules as positional arguments. Since we're likely to just use these one at a time in Checkbox jobs, should these be subparsers instead? If they are subparsers, then we could conceivably add the module configuration as options for those subparsers. The more I think about it, they should be subparsers. Let me know what you think!
I think the configs are complex enough to be kept into separated config files. I would restrict the modules
parameter to run only one module, and I think you could simplify the argparser by setting choices
for the possible modules:
parser.add_argument(
"modules",
metavar="MOD",
nargs="*",
choices=RVS_MODULES.keys(),
type=str,
help="RVS modules to run [{}]".format(", ".join(RVS_MODULES)),
)
Description
Resolved issues
Addresses CHECKBOX-969 and CHECKBOX-1539.
Documentation
Coverage tests are included with this PR.
Tests
I tested this on my desktop, which has an AMD GPU (
AMD Radeon RX 6750 XT
). NOTE: For some AMD GPUs, you may need to change the environment variableHSA_OVERRIDE_GFX_VERSION
forrvs
to recognize that your GPU is compatible with some ROCm/HIP functions. For example, for my GPU, I had to edit the value to10.3.0
.