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

docs: update cifar_brevitas_training accuracy using representative calibration set #235

Merged

Conversation

RomanBredehoft
Copy link
Collaborator

It turns out there was not a bug, this is the new accuracy we should talk about. Basically, we were not properly calibrating before, but more details can be found in https://github.com/zama-ai/concrete-ml-internal/issues/3957

closes https://github.com/zama-ai/concrete-ml-internal/issues/3957

@RomanBredehoft RomanBredehoft requested a review from a team as a code owner September 8, 2023 11:42
@cla-bot cla-bot bot added the cla-signed label Sep 8, 2023
| VGG FHE | 6 bits | 87.5\*\* |
| VGG FHE (simulation\*) | 8 bits | 88.0 |
| VGG FHE (simulation\*) | 7 bits | 87.2 |
| VGG FHE (simulation\*) | 6 bits | 86.0 |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the 5 bits because it gave ~60% accuracy, so not really relevant

| VGG FHE (simulation\*) | 8 bits | 88.0 |
| VGG FHE (simulation\*) | 7 bits | 87.2 |
| VGG FHE (simulation\*) | 6 bits | 86.0 |
| VGG FHE | 6 bits | 86.0\*\* |
Copy link
Collaborator Author

@RomanBredehoft RomanBredehoft Sep 8, 2023

Choose a reason for hiding this comment

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

I'm currently executing the FHE inference on 10 samples to check for that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@jfrery jfrery left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I think it's worth keeping a trace in this commit about why the accuracy changed since it's not in the same commit as the fix. Not sure where to add this.

@RomanBredehoft
Copy link
Collaborator Author

RomanBredehoft commented Sep 8, 2023

Thanks for the update. I think it's worth keeping a trace in this commit about why the accuracy changed since it's not in the same commit as the fix. Not sure where to add this.

a git blame on the readme should suffice no ?

EDIT: just understood your question actually, and Im also not sure how to do that (but not sure if it's really important)

Copy link
Collaborator

@kcelia kcelia left a comment

Choose a reason for hiding this comment

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

Thanks

@RomanBredehoft RomanBredehoft force-pushed the docs/update_cifar_brevitas_training_accuracy_readme_3957 branch from 1efa947 to 7a3839b Compare September 12, 2023 11:41
@RomanBredehoft RomanBredehoft changed the title chore: update cifar_brevitas_training accuracy results from the readme docs: update cifar_brevitas_training accuracy using representative calibration set Sep 12, 2023
@RomanBredehoft RomanBredehoft merged commit 39480ef into main Sep 12, 2023
@RomanBredehoft RomanBredehoft deleted the docs/update_cifar_brevitas_training_accuracy_readme_3957 branch September 12, 2023 13:49
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.

3 participants