Removes the generate thread #1054
Conversation
| total_processed = 0 | ||
| iteration_count = 0 | ||
|
|
||
| while not self._should_exit(): |
There was a problem hiding this comment.
similar question to above, what happens if some other process dies and the vllm worker should stop?
There was a problem hiding this comment.
Then it gets killed when Ray shuts down:
open-instruct/open_instruct/grpo_fast.py
Line 2708 in c3f79a3
| output.request_id, output, complete_output, current_time | ||
| ) | ||
|
|
||
| if self.verbose and iteration_count % 100 == 0: |
There was a problem hiding this comment.
i never really used this logging so fine with removing but was there any other impetus for doing so?
There was a problem hiding this comment.
Truthfully, I asked Claude to remove all the debug logging (I had a bunch of debug logging in an earlier version of this PR) and it also removed this. I thought it was fine so I kept it. Open to changing this if you prefer.
There was a problem hiding this comment.
no, all good! Was just wondering why the change.
The only reason we had the generate thread previously was to fix a locking issue that we had when updating the weights on the actor. Turns out that, with vLLM v1, the locking issue goes away!
Step time decreases significantly on the runs with
inflight_updates=True(wandb):Runs with
inflight_updates=True:Runs with
inflight_updates=False: