search

p-lambda / verified_calibration

Calibration library and code for the paper: Verified Uncertainty Calibration. Ananya Kumar, Percy Liang, Tengyu Ma. NeurIPS 2019 (Spotlight).
MIT License
142 stars 20 forks source link

Pickle trained Calibrators #5

Closed sudhanshu22gupta closed 3 years ago

sudhanshu22gupta commented 3 years ago

Trying to pickle a fitted HistogramMarginalCalibrator gives the error:

AttributeError: Can't pickle local object 'get_histogram_calibrator.<locals>.calibrator'

Jordy-VL commented 3 years ago

@sudhanshu22gupta managed to fix this one?

I think the solution is to wrap the class into an object:

class HistogramCalibrator(object):

AnanyaKumar commented 3 years ago

Sorry for the late reply - are you using Python 2 or Python 3? I believe Python 3 automatically adds object for you behind the scenes? (e.g. https://stackoverflow.com/questions/4015417/why-do-python-classes-inherit-object#:~:text=In%20Python%202%3A%20always%20inherit,for%20you%20behind%20the%20scenes.) I also tried pickling, and it doesn't give an error.

Warning: if you're pickling, do check if it pickles generally. I think for custom classes in general you may need to add additional methods to pickle properly. See https://stackoverflow.com/questions/10842553/pickle-with-custom-classes for more details. Lists may not get pickled automatically, but I haven't followed any more recent updates to the pickle package.

Feel free to send a PR with a few simple unit tests if you did get it working, I'm sure that'd be useful. And let us know if you have any other questions!

AnanyaKumar commented 3 years ago

Closing for now, but feel free to reopen if you have follow-up questions!

Jordy-VL commented 3 years ago

Hi Ananya,

Sorry for not responding sooner. For us pickle did not work due to the helper functions were in a separate function outside the class. However, we solved this by using dill instead of pickle. Nevertheless, for development purposes it is always better to make your class complete with all functionality embedded within. In this case I understand your design choice because the actual platt scaling is used in many different classes. Short answer: dill solved this issue, so it can remain closed.