Skip to content

Conversation

ferrine
Copy link
Member

@ferrine ferrine commented Jan 11, 2017

This PR implements auto minibatching with appropriate likelihood scaling. Now generator can be passed to data.

@ferrine
Copy link
Member Author

ferrine commented Jan 11, 2017

There is one but significant problem with generators. Any exception within calling __next__ can will break the generator inside theano.function

pymc3/model.py Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if population is the best name. population_size, total_size?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like both population and total_size. total size is probably more intuitive

pymc3/model.py Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs good doc-string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the casting should be necessary here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

This is really neat!

What happens if the generator raises an exception?

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

@twiecki then generator dies, I need to handle that case in some way or say user that it is dangerous. I think the latter is the only way I can solve the problem.

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

I can check if generator is picklable as such generators don't die when exception is caught

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

the second option is here (one more dependency and not always works)

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

"generator dies" -- raises another exception, crashes the kernel? how does it look like exactly?

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

@twiecki after exception generator raises StopIteration when __next__ is called, it becomes exhausted

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

I see, and in general this should never happen, the generators should be non-exhaustive. Thus, when that happens we can raise an exception that says that the generator stopped for unknown reasons and that we can't tell whether it was an exception. That seems acceptable to me.

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

KeyboardInterrupt can be annoing

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

Well, that's triggered by the user so it should be fairly obvious what happened in that case.

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

he will need to recompile the whole model

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

I don't think we need to stress over this. The generator will most likely be fast so the probability that the KeyboardInterrupt would happen there is low.

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

I'm stressed as risk is potentially high. Imagine DNN as a model. I would be disappointed

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

OK, how about we do not raise an exception but just display a warning. Question is what to do next, I suppose the only way to exit gracefully would be to raise a KeyboardInterrupt that will be caught by the inference algorithm which would be super odd. Or we add another Exception case to the inference to exist gracefully.

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

Is that a general constraint with generators? That exceptions do not percolate up?

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

These exceptions cannot be caught. If they appear inside __next__ it is fail in any case. KeyboardInterrupt is an exception that cannot be avoided

@twiecki
Copy link
Member

twiecki commented Jan 12, 2017

Well, that's a limitation of the language then. I think we should move ahead with raising an Exception of the generator stops. If it becomes annoying later on we can think of other options. I really don't think it's all that critical.

@ferrine
Copy link
Member Author

ferrine commented Jan 12, 2017

Good documentation is needed so

@twiecki
Copy link
Member

twiecki commented Jan 13, 2017

Agreed, documentation will mention that.

Do you instead want to merge this into your VI branch so that we can test how this would work in practice?

@ferrine
Copy link
Member Author

ferrine commented Jan 13, 2017

Yes, I'll add documentation and then merge

gladomat and others added 26 commits January 18, 2017 11:42
initialize chains from estimated posterior samples
Make init argument to sample case-insensitive
Added output for pointwise predictive accuracy in waic.
…tting (e.g. nanguardmode) for Theano functions
… mode setting (e.g. nanguardmode) for Theano functions"

This reverts commit 6bdb53a069c6d569fadcbc36a93489c0ddb334a3.
@ferrine ferrine closed this Jan 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants