Skip to content

Remove unused trace features #6188

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

Merged

Conversation

michaelosthege
Copy link
Member

@michaelosthege michaelosthege commented Oct 7, 2022

What is this PR about?
To prepare refactoring the sample storage backend, this PR simplifies things by removing unused features.

Starting chain numbering at values other than zero is a super rare edge case that isn't tested across all flavors of single/multiprocess sampling, and is likely to break downstream code.
Removing this feature simplifies the logic in sampling.py, thereby making it easier to refactor.

Secondly, I've removed merge_traces and the related merge_reports functions.
This is well covered by idata.extend these days.

Checklist

Major / Breaking Changes

  • None

Bugfixes / New features

  • None

Docs / Maintenance

  • Removed support for starting chain numbering at >0.
  • Removed non-public helper functions for merging MultiTrace objects.

@michaelosthege michaelosthege added maintenance major Include in major changes release notes section labels Oct 7, 2022
@michaelosthege michaelosthege self-assigned this Oct 7, 2022
@michaelosthege michaelosthege force-pushed the drop-edgecase-trace-features branch from 9714e3b to dcf67e9 Compare October 7, 2022 12:47
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #6188 (dcf67e9) into main (faebc60) will decrease coverage by 22.84%.
The diff coverage is 75.00%.

❗ Current head dcf67e9 differs from pull request most recent head 406f173. Consider uploading reports for the commit 406f173 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #6188       +/-   ##
===========================================
- Coverage   93.37%   70.52%   -22.85%     
===========================================
  Files         100      100               
  Lines       21896    21843       -53     
===========================================
- Hits        20445    15405     -5040     
- Misses       1451     6438     +4987     
Impacted Files Coverage Δ
pymc/backends/report.py 90.55% <ø> (-0.50%) ⬇️
pymc/parallel_sampling.py 76.49% <ø> (-9.32%) ⬇️
pymc/tests/backends/test_ndarray.py 100.00% <ø> (ø)
pymc/sampling.py 70.67% <66.66%> (-11.98%) ⬇️
pymc/backends/base.py 85.54% <100.00%> (-0.91%) ⬇️
pymc/tests/smc/test_smc.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/tuning/test_scaling.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/tuning/test_starting.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/step_methods/test_slicer.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/step_methods/hmc/test_nuts.py 0.00% <0.00%> (-100.00%) ⬇️
... and 31 more

@michaelosthege michaelosthege force-pushed the drop-edgecase-trace-features branch from dcf67e9 to 406f173 Compare October 7, 2022 13:16
@michaelosthege michaelosthege marked this pull request as ready for review October 7, 2022 14:06
@michaelosthege michaelosthege added major Include in major changes release notes section and removed major Include in major changes release notes section labels Oct 7, 2022
@ricardoV94 ricardoV94 removed the major Include in major changes release notes section label Oct 7, 2022
@ricardoV94 ricardoV94 merged commit 91dbfd2 into pymc-devs:main Oct 7, 2022
@michaelosthege michaelosthege deleted the drop-edgecase-trace-features branch October 7, 2022 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants