Skip to content
This repository was archived by the owner on Aug 7, 2025. It is now read-only.

Conversation

@lxning
Copy link
Collaborator

@lxning lxning commented Oct 28, 2020

Description

Please include a summary of the feature or issue being fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #(issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Feature/Issue validation/testing

Allow user to configure default batchSize and maxBatchDelay in properties when model is initiated.

Please describe the tests [UT/IT] that you ran to verify your changes and relevent result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.

  • Test A

  • Test B

  • UT/IT execution results

  • Logs

Checklist:

  • Have you added tests that prove your fix is effective or that this feature works?
  • New and existing unit tests pass locally with these changes?
  • Has code been commented, particularly in hard-to-understand areas?
  • Have you made corresponding changes to the documentation?

@lxning lxning requested a review from maaquib October 28, 2020 19:47
@sagemaker-neo-ci-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: torch-serve-build-cpu
  • Commit ID: 2a8c625
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-neo-ci-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: torch-serve-build-gpu
  • Commit ID: 2a8c625
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@lxning lxning requested a review from dk19y October 28, 2020 20:45
Copy link
Contributor

@harshbafna harshbafna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lxning

Not sure if it is a good idea to introduce a default batch_size and max_batch_delay parameter in TorchServe, as every model may use a different batch size.

There can be two more possible approaches for this :

  1. Allow user to provide these parameters as a part of model-archive creation
--batch_size 2 --max_batch_delay 50
  1. Allow users to specify batch_size and max_batch_delay with the load_models parameter. Something like:
load_models=model1:2:50,model2:5:100

@sagemaker-neo-ci-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: torch-serve-build-cpu
  • Commit ID: ac62167
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-neo-ci-bot
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: torch-serve-build-gpu
  • Commit ID: ac62167
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@lxning
Copy link
Collaborator Author

lxning commented Oct 30, 2020

@lxning

Not sure if it is a good idea to introduce a default batch_size and max_batch_delay parameter in TorchServe, as every model may use a different batch size.

There can be two more possible approaches for this :

  1. Allow user to provide these parameters as a part of model-archive creation
--batch_size 2 --max_batch_delay 50
  1. Allow users to specify batch_size and max_batch_delay with the load_models parameter. Something like:
load_models=model1:2:50,model2:5:100

Yeah, each model may have different batchSize and maxBatchDelay. Ideally, each model's manifest defines all the parameter's default values such as #workers, batchSize and maxBatchDelay. However, the existing implementation uses property file to define default #workers for all of the models. Each model's attributes (such as #workers, batchSize and maxBatchDelay) can be changed via http request. To follow existing implementation pattern, so I choose default value for all the models, instead of only define batchSize and maxBatchDelay in manifest.

@spate141
Copy link

@harshbafna any update on this?

@msaroufim msaroufim self-requested a review May 13, 2021 17:50
@msaroufim
Copy link
Member

@lxning should we go ahead and get this merged? There are a couple of issues asking for configuring batch sizes

Just to clarify which manifest should users change if they want to deploy several models at once each with their own batch size?

@msaroufim
Copy link
Member

Dupe of #1122

@msaroufim msaroufim closed this Jun 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

batch_size is hard coded as 1 if model is loaded during server initialization.

7 participants