add MT_BENCH to pipeline

[sallyom]

This adds MT_Bench - and uses ilab image with ilab model evaluate --benchmark mt_bench

[leseb]

It was not working in the first place because vLLM does not handle signals properly. It expects to be run from the CLI and thus stopped via SIGINT.

If you use a process group in the instantiation above you can get ride of the current pid search logic and simply do:

process_group_id = os.getpgid(process.pid)
process.send_signal(signal.SIGINT)
try:
    print("Waiting for vLLM server to shut down gracefully")
    process.wait(timeout)
except subprocess.TimeoutExpired:
    print(
        f"Sending SIGKILL to vLLM server since timeout ({timeout}s) expired"
    )
    process.kill()

# Attempt to cleanup any remaining child processes
# Make sure process_group is legit (> 1) before trying
if process_group_id and process_group_id > 1:
    try:
        os.killpg(process_group_id, signal.SIGKILL)
        print("Sent SIGKILL to vLLM process group")
    except ProcessLookupError:
        print("Nothing left to clean up with the vLLM process group")
else:
    print("vLLM process group id not found")

[sallyom]

Thanks! Please add in a follow-up, I need to be done with this PR 🤣

[sallyom]

@leseb how about if we merge this, then you add the PID/Kill changes in a follow-up?

[leseb]

@leseb how about if we merge this, then you add the PID/Kill changes in a follow-up?

Sounds good :).

[leseb]

Suggested change

	subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
	subprocess.Popen(args=command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

Why subprocess.PIPE? Both stdout and sterr are not used anywhere.

[sallyom]

relic from another try, another time - I'll remove those

[leseb]

can we enforce a particular version? @git+https://github.com/opendatahub-io/[email protected]

[tumido]

Or maybe bake it into the image? It's our custom image anyways... Installing in runtime without a lockfile is not safe.

[sallyom]

that was always my plan, but wasn't convenient while iterating on the 1000 different ways I tried to get vllm working properly - I'll add that to the Containerfile

[sallyom]

@tumido I'll update the Containerfile in a follow-up & remove the runtime install

[sallyom]

yes- will do that in a follow-up - will move the pip install to the Containerfile

[MichaelClifford]

LGTM