-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify BatchConfig #9
Comments
The new version of the
Field to remove:
Change name:
Add:
The new version of
Field to remove:
|
I would take the kernel part development. Got a question, |
|
Yes, we do can use If we switch to int index = 0;
while(request_idx) {
while(!request_available[index]){
++index;
}
--request_idx;
} And this would result into |
I agree with the idea of keeping this |
That's a valid concern. But I don't think we need to launch @aetiurf For this part, it seems we don't need to worry too much about this since |
Ah, I mix up the |
Actually it could be even smaller (e.g. 8 or 16). |
ANNOTATION: |
Is Please help me verify this :P |
In prefilling phase, |
As we plan to move some states from the
BatchConfig
to theRequestManager
, some fields inBatchConfig
are rendered redundant. The following are the data members of the currentBatchConfig
.The following fields seem redundant:
num_generation_tokens
: in previous versions, the prefilling and decoding phase of the small model is mixed, and this field is used to record the number of tokens generated by the small model in the decoding phase. Now we separate the prefilling and decoding phase of the small model, all tokens are generated in the decoding phase, and the number can be found innum_tokens
.PerRequestInfo.max_sequence_length
: I think this should be a field ofRequestManager
.PerRequestInfo.batch_config_request_id
: we can store a mapping from the index of a request in the batch to the guid of the request.request_completed
: This is stored inRequestManager
.request_running
: This is stored inRequestManager
.There are also some redundancies in the current
TreeSearchBatchConfig
, the following are the current data members:As the base class
BatchConfig
already has a field inPerRequestInfo
callednum_tokens_in_batch
, we don't need the structTreeSearchPerRequestInfo
, because the only field in it,num_tokens_at_depth
is equivalent tonum_tokens_in_batch
.Please let me know if any of the fields listed above is not redundant. Otherwise, let's remove them. We can discuss to move other fields into
RequestManager
if you have other suggestions.The text was updated successfully, but these errors were encountered: