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

Improve Camelyon16 benchmark #296

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Improve Camelyon16 benchmark #296

merged 3 commits into from
Jan 8, 2024

Conversation

xavier-owkin
Copy link
Contributor

@xavier-owkin xavier-owkin commented Jan 4, 2024

Fix the number of sampled tiles to 10 000 and ensure reproducibility by fixing torch RNG state

Results obtained with the fed_benchmark.py script:
New one
new
Old one
old

@xavier-owkin xavier-owkin requested a review from jeandut as a code owner January 4, 2024 15:07
@xavier-owkin xavier-owkin requested a review from a user January 4, 2024 15:07
@xavier-owkin xavier-owkin changed the title Improve benchmark Improve Camelyon16 benchmark Jan 4, 2024
@jeandut jeandut marked this pull request as draft January 4, 2024 15:16
Copy link
Collaborator

@jeandut jeandut left a comment

Choose a reason for hiding this comment

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

We would need to update the results running fed_benchmark on the new Camelyon16 and updates corresponding results (leaving a copy of the old one for legacy purposes) in this PR as well as its plot putting old and new results side by side.

@xavier-owkin xavier-owkin marked this pull request as ready for review January 4, 2024 15:51
@xavier-owkin xavier-owkin force-pushed the improve-benchmark branch 3 times, most recently from 4912af0 to 0541b4d Compare January 5, 2024 10:01
@xavier-owkin xavier-owkin force-pushed the improve-benchmark branch 4 times, most recently from 5f29aed to 07ecf9d Compare January 8, 2024 10:26
Copy link
Collaborator

@jeandut jeandut left a comment

Choose a reason for hiding this comment

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

Great work !
Can you:

  • rename all old files with the suffix "_old" including (results_benchmark_fed_camelyon.csv to results_benchmark_fed_camelyon_old.csv so the file should not be changed directly only copied)
  • new results are called like the old ones were before aka wo any suffix (no _vf)

Copy link
Collaborator

@jeandut jeandut left a comment

Choose a reason for hiding this comment

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

Great work ! Can you:

  • rename all old files with the suffix "_old" including (results_benchmark_fed_camelyon.csv to results_benchmark_fed_camelyon_old.csv so the file should not be changed directly only copied)
  • new results are called like the old ones were before aka wo any suffix (no _vf)

It seems I hallucinated on this one ! LGTM !

@xavier-owkin xavier-owkin merged commit ec5af8e into main Jan 8, 2024
5 checks passed
@xavier-owkin xavier-owkin deleted the improve-benchmark branch January 8, 2024 12:16
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.

2 participants