Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

find_MAP is failing in V4 #4771

Closed
ricardoV94 opened this issue Jun 14, 2021 · 7 comments · Fixed by #5445
Closed

find_MAP is failing in V4 #4771

ricardoV94 opened this issue Jun 14, 2021 · 7 comments · Fixed by #5445
Labels
Milestone

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Jun 14, 2021

While working on #4770 I found that find_MAP is failing quite often after the V4 refactoring.

Most issues seem related to gradients of discrete vars and/or models with both discrete and continuous unobserved variables. The failing tests in here are marked with an xfail for the time being.

@michaelosthege
Copy link
Member

Could it be a problem with the initialization too? (crossref #4752)
I found "find_MAP" to be quite brittle when the initialization is bad. Not only because optimization is hard, but also because the automatic convergence detection often makes it give up after a handful of iterations.

@ricardoV94
Copy link
Member Author

I have the impression it's more than that. But it was some time since I had a look

michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 17, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
This allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 17, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 17, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 18, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 18, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 19, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Jul 19, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
michaelosthege added a commit to michaelosthege/pymc that referenced this issue Aug 7, 2021
This changes the initval default on Distribution.__new__ and Distribution.dist to UNSET.
It allows for implementing distribution-specific initial values similar to how it was done in pymc3 <4.

Related to pymc-devs#4771.
@twiecki
Copy link
Member

twiecki commented Oct 27, 2021

@michaelosthege is this fixed now?

@twiecki twiecki closed this as completed Oct 27, 2021
@ricardoV94
Copy link
Member Author

This is not fixed. The tests marked with xfail above are still failing in main

@ricardoV94 ricardoV94 reopened this Oct 27, 2021
@michaelosthege
Copy link
Member

Yeah we need the get_moment implementations on the most important distributions plus the switch of the default strategy.

I opened #5087, but currently don't have any time to continue working on it for the near future. Anybody please feel free to take over.

@ricardoV94
Copy link
Member Author

ricardoV94 commented Oct 27, 2021

I double checked, the issue is not (just) the starting points. We are getting Type/Shape Errors somewhere inside the scipy optimization library

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jan 7, 2022

After #5320 there is only one test that is failing:

@pytest.mark.xfail(reason="first call to find_MAP is failing")

Some shape error comes from deep inside the scipy optimization routine. Only the first of the two calls to find_MAP seems to be raising, suggesting it is something to do with the exclusion of the discrete variable, which does not happen in the second call:

map_est1 = starting.find_MAP()
map_est2 = starting.find_MAP(vars=model.value_vars)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants