[CI][Benchmarks] Automatically detect component versions in benchmark CI #18339
Conversation
…hmark-ci-detect-version
| parser.add_argument( | ||
| "--detect-version-cpp-path", | ||
| type=Path, | ||
| help="Location of detect_version.cpp used to query e.g. DPC++, L0", | ||
| default=None, | ||
| ) |
There was a problem hiding this comment.
why does this need to be an argument? We know where this file resides (f"{os.path.dirname(__file__)}/utils/detect_versions.cpp").
There was a problem hiding this comment.
Mostly for incase it is not found where it should be / we want to supply a different one, but maybe I am overcomplicating this
There was a problem hiding this comment.
maybe I am overcomplicating this
;-)
We should do the absolute minimum that does the job.
| def get_compute_runtime_ver_cached(self) -> str: | ||
| return self.compute_runtime_ver_cache | ||
|
|
||
| def get_compute_runtime_ver(self) -> str: |
There was a problem hiding this comment.
can we set an env variable in the container with the correct compute runtime tag and only fallback to this expensive operation if that isn't set?
There was a problem hiding this comment.
We can, but then the question becomes how/where the value for that env variable comes from: I'm not actually sure where you'd have easy access to compute-runtime's version. We could set it manually and/or grep off of dependencies.json, but afaik the turnover for dependencies.json (i.e. if someone updates a field and a new image gets built) is daily: we could potentially have days of inaccurate compute-runtime versions.
To be honest, the proper solution should be a PR to L0 itself adding a builtin for compute-runtime version: I should not have to do all of this just to get a version in the first place.
There was a problem hiding this comment.
We could set it manually and/or grep off of dependencies.json
Yeah, I think that should be doable, it's just a nice string we can read directly:
Line 4 in 3a5ee6e
You can use jq to query this specific field. No need to grep anything.
There was a problem hiding this comment.
All in all, the operation isn't the slowest: it adds maybe about 1-2 seconds max if the compute-runtime version is found. Incase it is not immediately easy to find the compute-runtime version, that is what max_api_calls is for, in order to tweak how fast we want to give up.
There was a problem hiding this comment.
We could set it manually and/or grep off of dependencies.json
Yeah, I think that should be doable, it's just a nice string we can read directly:
Line 4 in 3a5ee6e
You can use jq to query this specific field. No need to grep anything.
In hindsight this would probably work fine for nightly, but the issue with dependencies.json being ahead of the version of compute-runtime used for the actual image would still hold true for manual dispatches.
The easiest way to go about this while also ensuring accuracy would probably be to:
- parse dependencies.json for a github tag
- check if L0 tag matches the tag in dependencies.json (probably 1 second max spent here)
- if it does not, we fetch the tag from the API, otherwise we default to dependencies.json
| def init(cls, detect_ver_path: Path, dpcpp_exec: str = "clang++"): | ||
| """ | ||
| Constructs the singleton instance for DetectVersion, and initializes by | ||
| building and run detect _version.cpp, which outputs: |
|
|
||
| # L0 version strings follows semver: major.minor.patch+optional | ||
| # compute-runtime version tags follow year.WW.patch.optional instead, | ||
| # but patch, pptional is still the same across both. |
| else: | ||
| return val | ||
| @staticmethod | ||
| def on_re(val: str, regex: re.Pattern, throw: Exception = None): |
There was a problem hiding this comment.
Looks like regex shall be of type str as all usages pass a raw string here, not a compiled pattern. Moreover, you do a regex compilation here in this function.
Previously, there was no way to figure out what version of compute-runtime we were using. This PR:
__clang_version__