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

Sample's step_kwargs argument doesn't recognize 'metropolis' #3197

Closed
moeyensj opened this issue Sep 16, 2018 · 6 comments
Closed

Sample's step_kwargs argument doesn't recognize 'metropolis' #3197

moeyensj opened this issue Sep 16, 2018 · 6 comments

Comments

@moeyensj
Copy link

Hello pymc3-devs and company,

Thank you for your time and effort.

I was trying to set the scaling for the Metropolis-Hastings sampler using the step_kwargs argument but it doesn't recognize the lower case name. It could also very well be that I am misunderstanding the docstring:

"""
step_kwargs : dict
        Options for step methods. Keys are the lower case names of the step method, values are
        dicts of keyword arguments. You can find a full list of arguments in the docstring of the
        step methods. If you want to pass arguments only to nuts, you can use `nuts_kwargs`.
"""

Here is a simplified example showing the problem:

import pymc3 as pm
import numpy as np

m = 2
b = 1.5
x = np.linspace(1, 10, 10)
Y = m*x + b

with pm.Model():
    b_m = pm.Uniform("b", lower=0, upper=1)
    m_m = pm.Uniform("m", lower=0, upper=1)
    y_m = pm.Normal("y", mu=m_m*x + b_m , observed=Y)
    
    step = pm.Metropolis()
    trace = pm.sample(draws=500, step=step, njobs=1, chains=10, step_kwargs={"metropolis": {"scaling": 100}})
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-1-7a473d66c703> in <module>()
     13 
     14     step = pm.Metropolis()
---> 15     trace = pm.sample(draws=500, step=step, njobs=1, chains=10, step_kwargs={"metropolis": {"scaling": 100}})

~/anaconda3/envs/py36/lib/python3.6/site-packages/pymc3/sampling.py in sample(draws, step, init, n_init, start, trace, chain_idx, chains, cores, tune, nuts_kwargs, step_kwargs, progressbar, model, random_seed, live_plot, discard_tuned_samples, live_plot_kwargs, compute_convergence_checks, use_mmap, **kwargs)
    413                 step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    414         else:
--> 415             step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    416 
    417         if isinstance(step, list):

~/anaconda3/envs/py36/lib/python3.6/site-packages/pymc3/sampling.py in assign_step_methods(model, step, methods, step_kwargs)
    149             selected_steps[selected].append(var)
    150 
--> 151     return instantiate_steppers(model, steps, selected_steps, step_kwargs)
    152 
    153 

~/anaconda3/envs/py36/lib/python3.6/site-packages/pymc3/sampling.py in instantiate_steppers(model, steps, selected_steps, step_kwargs)
     75     unused_args = set(step_kwargs).difference(used_keys)
     76     if unused_args:
---> 77         raise ValueError('Unused step method arguments: %s' % unused_args)
     78 
     79     if len(steps) == 1:

ValueError: Unused step method arguments: {'metropolis'}

This problem can be bypassed if I just instantiated the step method as follows step = pm.Metropolis(scaling=100) but I am trying to allow the users of the code I am working on to chose what step method they want to use and its kwargs.

Versions and main components

  • PyMC3 Version: 3.5
  • Theano Version: 1.0.2
  • Python Version: 3.6.6
  • Operating system: Mac OS 10.13.6
  • How did you install PyMC3: conda
@eigenfoo
Copy link
Member

@moeyensj thank you for the bug report!

After a bit of digging around, it looks like the issue stems from this block of code.

Essentially, the problem is that selected_steps does not contain variables that have already been assigned step methods through assigned_vars. This causes instantiate_steppers to raise the ValueError that you're getting. It looks to me that the easiest fix would simply be to add the assigned variables to the selected_steps dictionary, but one of the more experienced devs might know better.

I'll try and put together a patch sometime this week, but in the meantime, I can only suggest that you use the step = pm.Metropolis(scaling=100) syntax if you need an immediate workaround.

@eigenfoo
Copy link
Member

@moeyensj I just whipped up a PR to fix your issue; if all goes well it'll be merged soon 😄

@moeyensj
Copy link
Author

@eigenfoo Thanks so much!

I can fork the repo and test your PR if it helps at all.

@eigenfoo
Copy link
Member

The issue wasn't as easy a fix as I was expected: some tests aren't passing, and they look like real failures. I should be able to find time in the next week or so to patch it up, but if you're anxious/brave, feel free to fork my repo and finish it off! 😄

@sarajcev
Copy link

The same thing happens with the NUTS:

with pm.Model():
    b_m = pm.Uniform("b", lower=0, upper=1)
    m_m = pm.Uniform("m", lower=0, upper=1)
    y_m = pm.Normal("y", mu=m_m*x + b_m , observed=Y)
    
    step = pm.NUTS()
    trace = pm.sample(draws=500, step=step, chains=2, nuts_kwargs={"target_accept":0.95})

This is the error output:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-e008f07f8c35> in <module>()
      5 
      6     step = pm.NUTS()
----> 7     trace = pm.sample(draws=500, step=step, chains=2, nuts_kwargs={"target_accept":0.95})

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in sample(draws, step, init, n_init, start, trace, chain_idx, chains, cores, tune, nuts_kwargs, step_kwargs, progressbar, model, random_seed, live_plot, discard_tuned_samples, live_plot_kwargs, compute_convergence_checks, use_mmap, **kwargs)
    413                 step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    414         else:
--> 415             step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    416 
    417         if isinstance(step, list):

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in assign_step_methods(model, step, methods, step_kwargs)
    149             selected_steps[selected].append(var)
    150 
--> 151     return instantiate_steppers(model, steps, selected_steps, step_kwargs)
    152 
    153 

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in instantiate_steppers(model, steps, selected_steps, step_kwargs)
     75     unused_args = set(step_kwargs).difference(used_keys)
     76     if unused_args:
---> 77         raise ValueError('Unused step method arguments: %s' % unused_args)
     78 
     79     if len(steps) == 1:

ValueError: Unused step method arguments: set(['nuts'])

Python 2.7.15
PyMC3 Ver. 3.5

@sarajcev
Copy link

Also, consider using multiple samplers, for example:

with pm.Model():
    b_m = pm.Uniform("b", lower=0, upper=1)
    m_m = pm.Uniform("m", lower=0, upper=1)
    y_m = pm.Normal("y", mu=m_m*x + b_m , observed=Y)
    
    step_A = pm.Metropolis(vars=[b_m])
    step_B = pm.NUTS()
    trace = pm.sample(draws=500, step=[step_A, step_B], chains=2, step_kwargs={"metropolis": {"scaling": 100}})

This will throw the following error:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-4565eb706708> in <module>()
      6     step_A = pm.Metropolis(vars=[b_m])
      7     step_B = pm.NUTS()
----> 8     trace = pm.sample(draws=500, step=[step_A, step_B], chains=2, step_kwargs={"metropolis": {"scaling": 100}})

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in sample(draws, step, init, n_init, start, trace, chain_idx, chains, cores, tune, nuts_kwargs, step_kwargs, progressbar, model, random_seed, live_plot, discard_tuned_samples, live_plot_kwargs, compute_convergence_checks, use_mmap, **kwargs)
    413                 step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    414         else:
--> 415             step = assign_step_methods(model, step, step_kwargs=step_kwargs)
    416 
    417         if isinstance(step, list):

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in assign_step_methods(model, step, methods, step_kwargs)
    149             selected_steps[selected].append(var)
    150 
--> 151     return instantiate_steppers(model, steps, selected_steps, step_kwargs)
    152 
    153 

/home/ps/anaconda2/lib/python2.7/site-packages/pymc3/sampling.pyc in instantiate_steppers(model, steps, selected_steps, step_kwargs)
     75     unused_args = set(step_kwargs).difference(used_keys)
     76     if unused_args:
---> 77         raise ValueError('Unused step method arguments: %s' % unused_args)
     78 
     79     if len(steps) == 1:

ValueError: Unused step method arguments: set(['metropolis'])

Without using step_kwargs keyword arguments, it works just fine.

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

No branches or pull requests

3 participants