[benchmarking] Adds paths config for specifying Docker container mounts#1879
Conversation
…oints. Signed-off-by: rlratzel <rratzel@nvidia.com>
…mounts Signed-off-by: rlratzel <rratzel@nvidia.com>
Greptile SummaryThis PR replaces the flat Confidence Score: 4/5Safe to merge with minor follow-ups; all findings are non-blocking P2 suggestions. No P0 or P1 issues found. Three P2 items exist: the benchmarking/runner/utils.py — contains all three findings Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User / run.sh
participant GRV as gen_runscript_vars.py
participant MRG as merge_config_files()
participant VAL as assert_valid_config_dict()
participant PR as PathResolver
participant Docker as Docker CLI
User->>GRV: argv (--config files)
GRV->>MRG: config file paths
MRG-->>GRV: merged config_dict
GRV->>VAL: config_dict
VAL-->>GRV: ok / ValueError
GRV->>PR: config_dict
PR-->>GRV: PathResolver instance
GRV->>PR: volume_mount_pairs()
PR-->>GRV: (host_path, container_path) pairs
GRV->>Docker: --volume host:container flags
Note over GRV,VAL: run.py follows the same path via Session.from_dict
Reviews (7): Last reviewed commit: "Adds check to ensure paths is a list." | Re-trigger Greptile |
…re this. Signed-off-by: rlratzel <rratzel@nvidia.com>
…onfigs to be more robust, ensures paths sections don't have duplicate entries. Signed-off-by: rlratzel <rratzel@nvidia.com>
…mounts Signed-off-by: rlratzel <rratzel@nvidia.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
…mounts Signed-off-by: rlratzel <rratzel@nvidia.com>
…mounts Signed-off-by: rlratzel <rratzel@nvidia.com>
Signed-off-by: rlratzel <rratzel@nvidia.com>
This PR adds the
pathssection which defines files and directories used by the benchmarks.pathsreplaces the individualresults_path,datasets_path, andmodel_weights_pathsettings and allows users to set values that can be used by containers running on different machines with different host paths.Each
pathsentry requires anameand ahost_path, and all host paths must exist on the host. If running inside a Docker container started withtools/run.sh, each path is automatically mounted into the container. An optionalcontainer_pathoverrides the default mount point (default is host_path prefixed with/MOUNT), providing behavior similar to the previousresults_pathand others.Like
results_pathand others, the "name" can be used as a placeholder elsewhere in the config file (e.g.{datasets_path}) and will be substituted with eitherhost_pathorcontainer_pathbased on the environment.pathsis not limited to justresults_path,datasets_path, andmodel_weights_path; users can add additional mappings if needed, but definingresults_pathis required as it is used by the framework.If config files are encountered that use the deprecated
results_path,datasets_path, ormodel_weights_pathsettings, a warning will be printed. If a config (or series of merged configs) uses bothpathsand the legacy variables, an error will be raised.