FLI-based Worker Manager #622
Conversation
AlyssaCote
left a comment
AlyssaCote
left a comment
There was a problem hiding this comment.
This looks so, so good! Just a couple tiny comments. No hold ups from me, though!
| import dragon | ||
| from dragon import fli | ||
| except ImportError as exc: | ||
| if not "pytest" in sys.modules: |
There was a problem hiding this comment.
are you sure you want to look for pytest here? might be a copy/paste mistake
There was a problem hiding this comment.
It was an attempt to avoid failing tests when dragon was not available. This will be fixed once #621 will go in
| response = MessageHandler.deserialize_response(resp) | ||
| self.measure_time("deserialize_response") |
There was a problem hiding this comment.
Quick question. Shouldn't we be deserializing the request here? We serialize the request but I don't see where it's deserialized.
There was a problem hiding this comment.
Unless that's actually measured duringapp_receive, and if so never mind!
There was a problem hiding this comment.
I think on the app side we only want to deserialize the response - is there something I'm missing?
| timings.append(time.perf_counter() - interm) | ||
| interm = time.perf_counter() | ||
|
|
||
| print(" ".join(str(time) for time in timings)) |
There was a problem hiding this comment.
consider adding a custom log level e.g. "perf" so you can configure your logging output w/familiar logging interface / avoid prints
There was a problem hiding this comment.
That's an interesting suggestion, thanks
4 participants
This PR adds a simple
TorchWorkerwhich performs inference. The worker has only been tested for direct inference (and the filetest_torch_worker.pyreflects that). The output transform is still not implemented, but that's something that it is not needed for the moment being.