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 Minibatch alignment in Bayesian Neural Network example #689

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Deepakchowdavarapu
Copy link
Contributor

@Deepakchowdavarapu Deepakchowdavarapu commented Aug 2, 2024

Fix Minibatch Alignment in Bayesian Neural Network Example

Issue

Fixes #654.

Description

This PR addresses the issue where Minibatch variables for X_train and Y_train were created separately, causing potential misalignment in slices and thus random pairing of X with unrelated y.

Changes Made

  • Updated the code to create a single Minibatch for X_train and Y_train together, ensuring identical slices.
  • Modified the construction of ann_input and ann_output using the unified Minibatch variables.
  • Removed redundant parameters from the construct_nn function.
  • Confirmed model architecture and initialization remain consistent.

Impact

This change ensures that the model training process is more robust and prevents issues related to incorrect data alignment in minibatches.

Helpful links


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

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Deepakchowdavarapu
Copy link
Contributor Author

@yoshida-chem @twiecki please review waiting for your response

@twiecki
Copy link
Member

twiecki commented Aug 3, 2024

Thanks @Deepakchowdavarapu! You need to run pre-commit locally so that the myst file is generated.

@Deepakchowdavarapu
Copy link
Contributor Author

@twiecki ok sure can you please assign this issue to me !!!

@twiecki
Copy link
Member

twiecki commented Aug 4, 2024

There's no issue, on this PR you'd need to run pre-commit run --all-files and then commit and push.

@Deepakchowdavarapu
Copy link
Contributor Author

pre-commit.ci autofix

@twiecki
Copy link
Member

twiecki commented Aug 15, 2024

We might not have set that up here, but we should. Is pre-commit giving you problems?

@Deepakchowdavarapu
Copy link
Contributor Author

I will run it locally and raise a new PR soon.

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

Successfully merging this pull request may close these issues.

Bug in "Variational Inference: Bayesian Neural Networks" notebook
2 participants