Feature/SK-1081 | Use stores in Combiner + ModelPredict #718
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahellander
reviewed
Oct 8, 2024
hmm yes maybe |
Wrede
changed the title
Wrede
commented
Oct 11, 2024
fedn/network/clients/client.py
Outdated
| def _process_inference_request(self, model_id: str, session_id: str, presigned_url: str): | ||
| """Process an inference request. | ||
| def _process_prediction_request(self, model_id: str, session_id: str, presigned_url: str): | ||
| """Process an prediction request. |
| @@ -7,7 +7,7 @@ class ClientState(Enum): | |||
| idle = 1 | |||
| training = 2 | |||
| validating = 3 | |||
| inferencing = 4 | |||
| predicting = 4 | |||
There was a problem hiding this comment.
We can also add the ClientStateToString for "predicting"
fedn/network/clients/client.py
Outdated
|
|
||
| try: | ||
| _ = self.combinerStub.SendModelPrediction(prediction, metadata=self.metadata) | ||
| status_type = fedn.StatusType.INFERENCE |
There was a problem hiding this comment.
We can change StatusType.INFERENCE to StatusType.MODEL_PREDICTION in fedn.proto
fedn/network/combiner/combiner.py
Outdated
| :type model_id: str | ||
| :param config: the model configuration to send to clients | ||
| :type config: dict | ||
| :param clients: the clients to send the request to | ||
| :type clients: list | ||
|
|
||
| """ | ||
| clients = self._send_request_type(fedn.StatusType.INFERENCE, session_id, model_id, clients) | ||
| clients = self._send_request_type(fedn.StatusType.INFERENCE, prediction_id, model_id, clients) |
There was a problem hiding this comment.
see other comment about StatusType
ahellander
approved these changes
Oct 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes extensive changes to refactor the terminology from "inference" to "prediction" across the codebase, along starting using stores instead of mongostatestore.py in combiner. Adds ModelPredict message and SendModelPrediction method in proto to allow for Inference workflows. Also, keepalive arguments/options have been added for grpc channels and server. Below are the most important changes:
Breaking change:
endpoint /api/v1/infer -> /api/v1/predict
Terminology Refactor (Inference to Prediction)
.ci/tests/examples/prediction_test.py: Updated log messages to reflect the change from "inference" to "prediction"..ci/tests/examples/run_prediction.sh: Renamed script and updated curl commands and log messages to use "prediction" instead of "inference".examples/mnist-keras/client/predict.py: Changed comments and method calls from "inference" to "prediction".fedn/network/api/v1/__init__.py: Updated route imports to useprediction_routesinstead ofinference_routes.fedn/network/api/v1/prediction_routes.py: Added new prediction routes and removed the old inference routes.API Changes
fedn/network/clients/client.py: Refactored methods to handle prediction requests instead of inference requests, including logging and status updates. [1] [2] [3] [4] [5] [6] [7]fedn/network/combiner/combiner.py: Updated client handling and status management to reflect changes in prediction handling. [1] [2] [3]Minor Improvements and Bug Fixes
fedn/cli/status_cmd.py: Cleaned up docstring formatting.fedn/network/api/v1/status_routes.py: Added debug prints for typed headers and request arguments..github/workflows/integration-tests.yaml: Updated commented-out lines to reflect the change from "inference" to "prediction".fedn/network/clients/state.py: UpdatedClientStateenum to replace "inferencing" with "predicting".These changes ensure consistency in terminology and improve the clarity and functionality of the codebase.