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

Add a Gibbs sampler for the regularized logistic regression #13

Merged
merged 3 commits into from
Apr 1, 2022

Conversation

rlouf
Copy link
Member

@rlouf rlouf commented Feb 16, 2022

This PR adds a Gibbs sampler for the regularized binary logistic regression, as presented in Makalik & Schmidt (2016). The change from the regularized negative binomial regression is trivial, and I opened this PR to start thinking about how modular the implementation needs to be.

Closes #12, and we should aim to close #11 as well.

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR. If your commit description starts with "Fix...", then you're probably making this mistake.
  • There are tests covering the changes introduced in the PR.

@rlouf rlouf changed the title Add a Gibbs sampler for the regularized binary logistic regression Add a Gibbs sampler for the regularized logistic & linear regressions Feb 16, 2022
@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #13 (5e8171d) into main (f42e0e8) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #13   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           99       128   +29     
  Branches         6        10    +4     
=========================================
+ Hits            99       128   +29     
Impacted Files Coverage Δ
aemcmc/gibbs.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63a292c...5e8171d. Read the comment docs.

@rlouf rlouf force-pushed the binary-logistic-regression branch from d38fe00 to 880783f Compare March 23, 2022 15:15
@brandonwillard brandonwillard added the enhancement New feature or request label Mar 23, 2022
@brandonwillard brandonwillard force-pushed the binary-logistic-regression branch from 880783f to 6a70509 Compare March 23, 2022 21:45
@brandonwillard brandonwillard requested a review from zoj613 March 23, 2022 21:57
brandonwillard
brandonwillard previously approved these changes Apr 1, 2022
Copy link
Member

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this refactoring. Just don't be surprised if it's refactored again soon, because this project is likely to undergo large changes quickly.

@zoj613
Copy link
Member

zoj613 commented Apr 1, 2022

Is this ready for review or should I wait till its no longer in draft state? @rlouf

@rlouf rlouf changed the title Add a Gibbs sampler for the regularized logistic & linear regressions Add a Gibbs sampler for the regularized logistic regression Apr 1, 2022
@rlouf rlouf force-pushed the binary-logistic-regression branch from 6a70509 to 0cb5040 Compare April 1, 2022 08:15
@rlouf rlouf marked this pull request as ready for review April 1, 2022 08:16
@rlouf
Copy link
Member Author

rlouf commented Apr 1, 2022

@zoj613 Feel free to review while I pull the changes from main. The style checks are failing because of a problem with the version of black used in pre-commit. I bumped the version in #18 .

@rlouf rlouf force-pushed the binary-logistic-regression branch 2 times, most recently from b686e1e to 5e8171d Compare April 1, 2022 09:00
@rlouf
Copy link
Member Author

rlouf commented Apr 1, 2022

@zoj613 All checks pass now. Waiting for your review to merge.

@rlouf rlouf requested a review from brandonwillard April 1, 2022 14:56
@rlouf rlouf merged commit 165056a into aesara-devs:main Apr 1, 2022
@rlouf rlouf deleted the binary-logistic-regression branch April 1, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement sampler for logistic regularized regression Implement sampler for linear regularized regression
3 participants