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

Batchbald_implementation #133

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AnkitaKumariJain14
Copy link
Contributor

@AnkitaKumariJain14 AnkitaKumariJain14 commented Jul 15, 2022

Description

Notebook for implementation of BatchBALD and BALD algorithms.

Figure Number

Figures

image
image

Issue

Steps

Checklist:

  • Performed a self-review of the code
  • Tested on Google Colab.

Potential problems/Important remarks

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -0,0 +1,395 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #10.    from probml_utils import savefig, latexify

Can you add try.. except blocks wherever applicable? Checkout contributor guidelines if you are unsure.


Reply via ReviewNB

@@ -0,0 +1,395 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #1.    latexify(width_scale_factor=2, fig_height=2)

Can you adjust this as the following logic?

If a figure is covering 1/3 of the page width, we should set width_scale_factor=3.


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is set to 2 because it generates only 2 figures. The accuracy plot and the visualisation for the digits picked.

@@ -0,0 +1,395 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #17.    savefig("accuracy")

Can you give a better explicit name to figure?


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to test_accuracy_curve. Does it work?

@@ -0,0 +1,395 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #2.    plt.rcParams["axes.titlesize"] = 10

Why size 10? we are keeping max size 9 because it matches with caption.


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Size 10 seemed to match for the figures generated. I'll change it to 9

@patel-zeel
Copy link
Collaborator

I have given some comments here @AnkitaKumariJain14.

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.

None yet

3 participants