Skip to content
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

Batch processor enhancemenst through raw data parameter #3718

Merged
merged 10 commits into from
Nov 5, 2021
Merged

Batch processor enhancemenst through raw data parameter #3718

merged 10 commits into from
Nov 5, 2021

Conversation

RafalSkolasinski
Copy link
Contributor

@RafalSkolasinski RafalSkolasinski commented Nov 2, 2021

What this PR does / why we need it:

These solves family of issues gathered together under #3702

Which issue(s) this PR fixes:

Closes #3702

and therefore as well

Closes #2657
Closes #3409
Closes #3681
Closes #3408

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@RafalSkolasinski
Copy link
Contributor Author

RafalSkolasinski commented Nov 2, 2021

Example on echo model for input:

{"data": {"names": ["a", "b", "c"], "ndarray": [[1, 2, 3]]}, "meta": {"tags": {"customer-id": 0}}}
{"data": {"names": ["a", "b", "c"], "ndarray": [[4, 5, 6]]}, "meta": {"tags": {"customer-id": 1}}}
{"data": {"names": ["a", "b", "c"], "ndarray": [[7, 8, 9]]}, "meta": {"tags": {"customer-id": 2}}}
{"data": {"names": ["a", "b", "c"], "ndarray": [[1, 3, 6]]}, "meta": {"tags": {"customer-id": 3}}}

run as

export SELDON_BATCH_WORKERS=100
export SELDON_BATCH_RETRIES=3

export SELDON_BATCH_HOST="localhost:9000"
export SELDON_BATCH_GATEWAY_TYPE=seldon

export SELDON_BATCH_DEPLOYMENT_NAME=model

export SELDON_BATCH_METHOD=predict
export SELDON_BATCH_TRANSPORT=rest

export SELDON_BATCH_DATA_TYPE=raw

export SELDON_BATCH_INPUT_DATA_PATH=input-new.txt
export SELDON_BATCH_OUTPUT_DATA_PATH=predict-output-new.txt

export SELDON_BATCH_SIZE=1

seldon-batch-processor

will give as output

{"data": {"names": ["t:0", "t:1", "t:2"], "ndarray": [[1, 2, 3]]}, "meta": {"tags": {"batch_id": "3eea7a8c-3bce-11ec-93bd-f799141f921b", "extra-tag": 10, "customer-id": 0, "batch_index": 0, "batch_instance_id": "3eeaad5e-3bce-11ec-93bd-f799141f921b-item-0"}}}
{"data": {"names": ["t:0", "t:1", "t:2"], "ndarray": [[4, 5, 6]]}, "meta": {"tags": {"batch_id": "3eea7a8c-3bce-11ec-93bd-f799141f921b", "extra-tag": 10, "customer-id": 1, "batch_index": 1, "batch_instance_id": "3eeaad5e-3bce-11ec-93bd-f799141f921b-item-1"}}}
{"data": {"names": ["t:0", "t:1", "t:2"], "ndarray": [[7, 8, 9]]}, "meta": {"tags": {"batch_id": "3eea7a8c-3bce-11ec-93bd-f799141f921b", "extra-tag": 942, "customer-id": 2, "batch_index": 2, "batch_instance_id": "3eeaad60-3bce-11ec-93bd-f799141f921b-item-0"}}}
{"data": {"names": ["t:0", "t:1", "t:2"], "ndarray": [[1, 3, 6]]}, "meta": {"tags": {"batch_id": "3eea7a8c-3bce-11ec-93bd-f799141f921b", "extra-tag": 942, "customer-id": 3, "batch_index": 3, "batch_instance_id": "3eeaad60-3bce-11ec-93bd-f799141f921b-item-1"}}}

(the extra-tag is randomly assigned from model)

@RafalSkolasinski
Copy link
Contributor Author

/test integration

Copy link
Contributor

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

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

Nice one - looks good overall, added couple of comments, would be good to also add documenattion as part of the PR in the batching section to cover the constraints in which this would work as weell

python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
python/seldon_core/batch_processor.py Show resolved Hide resolved
python/seldon_core/batch_processor.py Show resolved Hide resolved
@RafalSkolasinski
Copy link
Contributor Author

/test integration

Copy link
Contributor

@axsaucedo axsaucedo left a comment

Choose a reason for hiding this comment

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

NIce one - comments are quite minor and some are more suggestions so we can resolve if not relevant, and we can merge once tests pass
/approve

python/seldon_core/batch_processor.py Show resolved Hide resolved
python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
python/seldon_core/batch_processor.py Outdated Show resolved Hide resolved
doc/source/servers/batch.md Show resolved Hide resolved
doc/source/servers/batch.md Show resolved Hide resolved
@RafalSkolasinski
Copy link
Contributor Author

/retest

@RafalSkolasinski
Copy link
Contributor Author

/test integration

@RafalSkolasinski
Copy link
Contributor Author

/retest

@RafalSkolasinski
Copy link
Contributor Author

/test integration

@RafalSkolasinski
Copy link
Contributor Author

Integration failure on: TestPrepack.test_sklearn_v2 so unrelated to the PR

@RafalSkolasinski
Copy link
Contributor Author

The executor failure

--- FAIL: TestTimeout (1.00s)
    client_test.go:315: Started
    client_test.go:341: 
        Expected
            <string>: Get "http://127.0.0.1:35239/health/status": context deadline exceeded
        to contain substring
            <string>: Client.Timeout exceeded while awaiting headers

looks odd but if anything this is unrelated to this PR.

@axsaucedo
Copy link
Contributor

/approve

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: axsaucedo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

stephen37 pushed a commit to stephen37/seldon-core that referenced this pull request Dec 21, 2021
* Add predict raw functionality to batch processor

* linter

* fix 1

* refactor extracting mini-batch from raw payloads into separate function

* remove all_equal function

* improve logging of exceptions

* further improve logging

* final improvements + documentation

* add typing; change back to indexes

* add link to issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants