[ROCm][CI] Revamping AMD mirrors#35897
[ROCm][CI] Revamping AMD mirrors#35897AndreasKaratzas wants to merge 8 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
There was a problem hiding this comment.
Code Review
This pull request attempts to fix a quotation issue by stripping surrounding double quotes from a variable. However, the implementation introduces code duplication in two places within the re_quote_pytest_markers function. My review includes a suggestion to refactor this duplicated logic into a reusable shell function to improve code maintainability.
| if [[ "$marker_buf" == '"'*'"' ]]; then | ||
| marker_buf="${marker_buf#\"}" | ||
| marker_buf="${marker_buf%\"}" | ||
| fi |
There was a problem hiding this comment.
This logic for stripping surrounding quotes is duplicated later in the script (lines 253-256). To improve maintainability and avoid code duplication, consider extracting this logic into a reusable shell function.
For example, you could define a function at the start of re_quote_pytest_markers:
_strip_surrounding_quotes() {
local s=$1
if [[ "$s" == '"'*'"' ]]; then
s="${s#\"}"
s="${s%\"}"
fi
echo "$s"
}Then you can replace this block and the duplicated one with a call to this function:
marker_buf=$(_strip_surrounding_quotes "$marker_buf")There was a problem hiding this comment.
DRYing the code in this point is pointless. This script will and has to heavily change. We are simply putting some patches in place to accelerate the gating test process.
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
|
Manually cancelled the runs to save resources. Will re-enable ready label and CI runs when the aforementioned dependencies are resolved. |
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
|
Observed 2 failures:
The above comments were made based on: https://buildkite.com/vllm/ci/builds/56267/steps/canvas |
|
Update on AMD: Language Models Test (Extended Pooling) (mi250_1) Looks like it was an intermittent failure: https://buildkite.com/vllm/amd-ci/builds/6559/steps/canvas?sid=019cf53b-06a0-4731-aa2a-420b95f27459&tab=output Will be monitoring in the upcoming nightlies as well as the upcoming re-evals of this PR. |
|
I also put a PR up to stabilize the RemoteOpenAIServer class: |
|
This pull request has merge conflicts that must be resolved before it can be |
In this PR, we are refining gating tests to utilize MI250 agents as well. I've put this PR up to start the evaluation of these tests and get tests ready for merging as soon as issues are confirmed to be resolved.
Mirrored AMD Tests
Total mirrored AMD tests: 30
Device Breakdown
Dependencies:
cc @kenroche