Initial draft of version of model_builder to work with scikit-learn.

[pdb5627]

Resolves #155

[twiecki]

This looks great! I wonder if perhaps we should just make the base class directly use this API. But it certainly can't hurt to also try it out here first.

[michaelraczycki]

Nice job on the fit refactoring! From what I see the model_builder was already compatible with scikit-learn or you didn't finish pushing the changes yet?

[pdb5627]

Nice job on the fit refactoring! From what I see the model_builder was already compatible with scikit-learn or you didn't finish pushing the changes yet?

@michaelraczycki No, it wasn't compatible. I made a new file bayesian_estimator_linearmodel.py for a subclass of ModelBuilder and an example implemention subclass. I introduced new tests in a new file tests/test_bayesian_estimator_linearmodel.py. The refactoring in ModelBuilder was to put the sampling into a function that wouldn't be overridden in the subclass. I also pulled out the attr-setting code so that it could be reused by a function for sampling the prior predictive distribution.

[pdb5627]

Here's a comparison of the APIs:

ModelBuilder	BayesianEstimator	Comment
init	init	Remove data from constructor. Default args come from properties.
	default_model_config ***
	default_sampler_config ***
	_validate_data
_data_setter ***	_data_setter ***	Adapted for X,y rather than data
create_sample_input ***	create_sample_input ***	Creates just data, not configuration. Default args from properties.
build_model ***	build_model ***	Takes no arguments. Instead of setting MutableData values, puts in dummy data of the right number of dimensions.
sample_model
save
load	load	Adapted for different constructor arguments
fit	fit	Adapted for X,y rather than data
predict	predict	Adapted for X,y rather than data
predict_posterior	predict_posterior	Adapted for X,y rather than data
	predict_proba
	sample_prior_predictive
	sample_posterior_predictive
_serializable_model_config
id

*** = abstract method requiring implementation in subclasses.

[michaelraczycki]

Oh, I focused much more on the changes you've introduced to model_builder itself, I didn't go into the implementation of Bayesian Estimator that much in detail as It doesn't affect other projects so far. I really like the ideas you're proposing, could we schedule a short call to discuss Bayesian Estimator a bit in depth? I'm working on making the model_builder to a standard used by other pymc-related projects, and I wanted to discuss with you which of the API changes you've suggested are complete must-have for sklearn compatibility

[pdb5627]

Oh, I focused much more on the changes you've introduced to model_builder itself, I didn't go into the implementation of Bayesian Estimator that much in detail as It doesn't affect other projects so far. I really like the ideas you're proposing, could we schedule a short call to discuss Bayesian Estimator a bit in depth? I'm working on making the model_builder to a standard used by other pymc-related projects, and I wanted to discuss with you which of the API changes you've suggested are complete must-have for sklearn compatibility

@michaelraczycki Ok, sounds good. How would you like to set it up? I'm in UTC+3.

[michaelraczycki]

I'm in UTC+0, so anything after your lunch should work. My email is [email protected], if you shoot me a message with some time slots that suit you I can create a google meeting

[michaelraczycki]

@pdb5627 is there something else you'd like to add here?

[michaelraczycki]

Amazing job, thank you for your contribution!

[twiecki]

Indeed, this is great @pdb5627!

[pdb5627]

Thanks, guys. I'm glad I could contribute.