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

Fix progressbar bug for parallel SMC sampling #7079

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

jucor
Copy link
Contributor

@jucor jucor commented Dec 29, 2023

See discussion in #7078 (comment)

Description

As discussed in #7078, running parallel SMC sampling (cores > 1) from within a notebook crashes. The package progressbar throws an error because it is not finding HTML. That error only occurs when running multiple chains in parallel from within a jupyter notebook.

This is because the progressbar module conditionally imports HTML at import time if it detects that it is imported from a notebook process, which is the case when the progress bar is created before the multi-chain forking. When the subprocess corresponding to one chain then tries to update the progress bar, it does so from a process that was not a notebook process when it imported the progressbar module after forking, and thus HTML is not imported.

The simplest fix, that I have implemented, is to force the use of a non-HTML (aka console, i.e. text mode) progressbar when sample_smc is running parallel chains. 2-lines fix :)

While I discovered this bug while investing another behaviour in #7078, it is also the underlying issue to #5855 and #5980, which were closed but not really fixed.

I wish I could add a unit test, but not easy way as this behavior occurs only when pymc.smc_sample is called from within a jupyter notebook. A test would therefore have to run a jupyter server then run a notebook then catch the error.

No documentation update needed.

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7079.org.readthedocs.build/en/7079/

Copy link

welcome bot commented Dec 29, 2023

Thank You Banner
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

Copy link

codecov bot commented Dec 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (118be0f) 92.21% compared to head (843ad7c) 92.21%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7079   +/-   ##
=======================================
  Coverage   92.21%   92.21%           
=======================================
  Files         101      101           
  Lines       16912    16913    +1     
=======================================
+ Hits        15595    15596    +1     
  Misses       1317     1317           
Files Coverage Δ
pymc/smc/sampling.py 99.30% <100.00%> (+<0.01%) ⬆️

@ricardoV94 ricardoV94 added maintenance SMC Sequential Monte Carlo labels Dec 31, 2023
@jucor
Copy link
Contributor Author

jucor commented Jan 20, 2024

👋 Hi @aloctavodia, Sorry to bother you, would you have any time for reviewing this one-line change, please?
Thanks @twiecki !

@twiecki twiecki merged commit e3f008a into pymc-devs:main Jan 21, 2024
23 checks passed
Copy link

welcome bot commented Jan 21, 2024

Congratulations Banner]
Congrats on merging your first pull request! 🎉 We here at PyMC are proud of you! 💖 Thank you so much for your contribution 🎁

@twiecki
Copy link
Member

twiecki commented Jan 21, 2024

👋 Hi @aloctavodia, Sorry to bother you, would you have any time for reviewing this one-line change, please?
Thanks @twiecki !

Thanks for the contribution!

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

Successfully merging this pull request may close these issues.

name 'HTML' is not define with pm.sample_smc() NameError: name 'HTML' is not defined
3 participants