Repository navigation
fix(models): leave the scored splits out of a saved model - #950
Open
Felipedino wants to merge 1 commit into
Open
Felipedino wants to merge 1 commit into
Felipedino wants to merge 1 commit into
Conversation
Whoever fits a model points x_data and y_data at the splits it trains on, so calculate_metrics can read them. The splits are in-memory Arrow tables, and every model that saves itself with joblib.dump(self) wrote them into its artifact: a copy of train, validation and test on every run, and a save that grew with the dataset instead of with the model. BaseModel.__getstate__ now drops both attributes, which covers every caller of model.save (SaveModelUnit and the pipeline Train node) at once. BaseModel declares both as None, so a loaded model, like a model never pointed at any data, scores nothing on its own instead of raising AttributeError. FaissBaseModel built its state from __dict__ directly and now goes through super(), so FAISS models drop the data too. Artifacts saved before this still load; they only carry the extra data.
This branch has not been deployed
This file contains hidden or 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
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.
Problem
When a model is fitted, its train/validation/test splits are attached to it as
x_data/y_dataso thatcalculate_metricscan read them. Those splits are in-memory Arrow tables. Every model that saves itself withjoblib.dump(self)therefore wrote the whole dataset into its.joblib. On large datasets,SaveModelUnittook about a minute, and every run kept its own copy of the data.Fix
BaseModel.__getstate__dropsx_data/y_datafrom the pickled state. This covers every caller ofmodel.save, including the pipelineTrainnode.BaseModeldeclares both attributes asNone, so a loaded model behaves like one that was never given data.FaissBaseModel.__getstate__now callssuper(), so FAISS models drop the data too.The model in memory is unchanged. Artifacts saved before this fix still load; they just keep the extra data.
Tests
New
tests/back/models/test_saved_model_leaves_out_data.py(sklearn and FAISS KMeans): the restored model has no data, the artifact size does not grow with the data, the live model keeps its data, predictions match after reload, and a model with no attributes can still be pickled. 5 of its 9 tests fail without the fix.