VowpalWabbit / py-vowpal-wabbit-next

Experimental new Python bindings for the VowpalWabbit library
https://vowpal-wabbit-next.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
12 stars 5 forks source link

feat: load model from path #95

Closed bassmang closed 1 year ago

jackgerrits commented 1 year ago

Is there a benefit of this over loading the file in user code?

The reason serialize_to_file was added was the address a performance issue raised in #84. The performance of loading a model was adequate and doesn't suffer the same issues as serialization (continually resizing a vector). To avoid interface bloat I think we should strive to avoid multiple ways of doing things when there isn't clear benefit/reason.

bassmang commented 1 year ago

Is there a benefit of this over loading the file in user code?

The reason serialize_to_file was added was the address a performance issue raised in #84. The performance of loading a model was adequate and doesn't suffer the same issues as serialization (continually resizing a vector). To avoid interface bloat I think we should strive to avoid multiple ways of doing things when there isn't clear benefit/reason.

with open("model.bin", "rb") as f:
     model_data = f.read()
workspace = Workspace(model_data=model_data)

This just seems a bit clunky/unintuitive from a user's perspective for a common operation. Maybe a load_model function to avoid interface bloat?