Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (18)
@twiecki twiecki Feb 4, 2020
```suggestion - `DEMetropolisZ`, an improved variant of `DEMetropolis` brings better parallelization and higher efficiency with fewer chains with a slower initial convergence. This implementation is experimental. See [#3784](https://github.com/pymc-devs/pymc3/pull/3784) for more info. ```
Outdated
RELEASE-NOTES.md
@aloctavodia aloctavodia Feb 4, 2020
Shouldn't be "DEMetropolisZ can achieve the same effective sample sizes with less chains."
Outdated
.../DEMetropolisZ_EfficiencyComparison.ipynb
@aloctavodia aloctavodia Feb 4, 2020
same as my other comment
Outdated
.../DEMetropolisZ_EfficiencyComparison.ipynb
@aloctavodia aloctavodia Feb 4, 2020
I think this import is not used. Also could we follow the convention of using alias when importing the libraries like `import numpy as np`, I think this is the style we follow in most notebooks.
Outdated
...ks/DEMetropolisZ_tune_drop_fraction.ipynb
michaelosthege
Michael Osthege
@twiecki twiecki Feb 3, 2020
Need link.
Outdated
RELEASE-NOTES.md
@twiecki twiecki Feb 3, 2020
```suggestion - `DEMetropolisZ`, an improved variant of `DEMetropolis` brings better parallelization and high efficiency with few chains with a slower initial convergence. This implementation is experimental. See [#3784](https://github.com/pymc-devs/pymc3/pull/3784) for more info. ```
Outdated
RELEASE-NOTES.md
@canyon289 canyon289 Feb 3, 2020
Being picky here. This test would be easier to read if written like this ``` tune =300 tune_drop_fraction=.85 draws=200 step = DEMetropolisZ(tune_drop_fraction=tune_drop_fraction) trace = sample( tune=tune, draws=draws, step=step, cores=1, chains=1, discard_tuned_samples=False ) assert len(step._history) == (tune - tune * tune_drop_fraction) + draws ``` That way there's less mental steps to correlate where the step_history expectation is coming from. Same for trace
Outdated
pymc3/tests/test_step.py
@canyon289 canyon289 Feb 3, 2020
Same comment here about parameterization It's worth parameterizing this so in the vent multiple tests fail its easier to figure out which ones are failing. Right now earlier test failures will hide the failures of the subsequent ones
Outdated
pymc3/tests/test_step.py
@canyon289 canyon289 Feb 3, 2020
It's worth parameterizing this so in the vent multiple tests fail its easier to figure out which ones are failing. Right now earlier test failures will hide the failures of the subsequent ones
Outdated
pymc3/tests/test_step.py
@canyon289 canyon289 Feb 3, 2020
What is this pass doing?
Outdated
pymc3/tests/test_step.py
canyon289 michaelosthege
Ravin Kumar and Michael Osthege
@canyon289 canyon289 Feb 3, 2020
Can this be made a standard triple quotes comment? That way it shows up with `help()` and doc discovery tools, such as the ones in ides
Outdated
pymc3/step_methods/metropolis.py
@canyon289 canyon289 Feb 3, 2020
What's 2.38? Can a comment be left for the magic number?
Outdated
pymc3/step_methods/metropolis.py
aloctavodia
Osvaldo A Martin
@canyon289 canyon289 Feb 3, 2020
This is very picky; Would be easier to read if written `if tune not in {None, 'scaling', 'lambda'}:` because then it reads like english instead of like a program. Not implying that a change is required
Outdated
pymc3/step_methods/metropolis.py
@junpenglao junpenglao Jan 21, 2020
Could we use a more informative name?
Outdated
pymc3/step_methods/metropolis.py
junpenglao aloctavodia
michaelosthege
Junpeng Lao, Osvaldo A Martin, and Michael Osthege
@aloctavodia aloctavodia Jan 21, 2020
what about `iz1, iz2 = np.random.choice(range(it+1), size=2, replace=False)`
Outdated
pymc3/step_methods/metropolis.py
aloctavodia michaelosthege
Osvaldo A Martin and Michael Osthege
@junpenglao junpenglao Jan 21, 2020
Is there any concern for memory for large num_step? It would be great if we can clear out `self._history` if user are calling from `pm.sample`
Outdated
pymc3/step_methods/metropolis.py
michaelosthege
Michael Osthege
@junpenglao junpenglao Jan 21, 2020
Should `self._it` and `self._history` also be reset after tuning?
Outdated
pymc3/step_methods/metropolis.py
michaelosthege junpenglao
Michael Osthege and Junpeng Lao
@junpenglao junpenglao Jan 21, 2020
need an Experimental warning.
Outdated
pymc3/step_methods/metropolis.py