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.

Sign up for GitHub

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

Jump to bottom

Use scikit-learn confusion matrix instead of own custom code #20

Merged
merged 27 commits into from
Nov 10, 2021
Merged

Use scikit-learn confusion matrix instead of own custom code #20

merged 27 commits into from
Nov 10, 2021

Conversation

jo-mueller
Copy link
Contributor

Enhanced overlap matrix calculation with sklearn implementation

The calculation of the label overlap matrix has been exchanged with the implementation from sklearn, which is considerably faster. After the upgrade, the benchmark (/notebooks/label/overlap_matrix_benchmarking.ipynb)) shows comparable results for both approaches.

Type of change

  • Bug-fix
  • New feature
  • Breaking change
  • Documentation update

References

Closes #3

Tests

  • I adapted existing tests, because
  • I added new tests to cover the code change
  • All tests pass with my change

Final checks

  • My change is the minimal possible work for the desired feature/fix
  • I updated the documentation where necessary to cover the change

haesleinhuepf and others added 20 commits November 7, 2021 12:18
@haesleinhuepf
benchmark overlap matrix generation
803e31c
@haesleinhuepf
typos
3f8492f
@jo-mueller
Update _label_overlap_matrix.py
5acfbaf
@jo-mueller
Added sklaern to core dependencies
bd0569b
@jo-mueller
Updated test
fdb94d6
@jo-mueller
updated tests
9d58b29
@jo-mueller
Adjusted biapolutils name in import
33c3ca6
@jo-mueller
added sklearn version and changed old function version to legacy func…
a7d00ae 
…tion
@jo-mueller
Added tests for sklearn label overlap function
e606f60
@jo-mueller
Bugfix in test
5e1674c
@jo-mueller
cosmetic changes
1d76c7c
@jo-mueller
added sequential labelling for robustness
308be97
@jo-mueller
Added old overlap_matrix as legac function
22a8b2a
@jo-mueller
Update _label_overlap_matrix.py
78fb286
@jo-mueller
Fixed test
89cecc4
@jo-mueller
removed legacy label_overlap function
54ba14b
@jo-mueller
Removed legacy overlap function from init.py
437f46b
@jo-mueller
removed legacy overlap function from IoU script
4b5ccc4
@jo-mueller
re-performed benchmark
c1c6fc7
@jo-mueller
Added info to description
59cfa20
@jo-mueller jo-mueller mentioned this pull request Nov 9, 2021
Closed
haesleinhuepf
haesleinhuepf requested changes Nov 9, 2021
View reviewed changes
Copy link
Member

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

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

Hi Johannes @jo-mueller ,

just some minor stuff and I'm suggesting to remove one function. Furthermore, could you please also delete overlap_matrix_benchmarking.ipynb? I created this branch with that notebook just to demonstrate something. I wasn't thinking of making this notebook part of the repository.

I'm happy to merge your changes afterwards.

Thanks!

Best,
Robert

biapol_utilities/label/_label_overlap_matrix.py Outdated Show resolved Hide resolved
biapol_utilities/label/_label_overlap_matrix.py Outdated Show resolved Hide resolved
biapol_utilities/label/_match_labels.py Show resolved Hide resolved
biapol_utilities/label/_match_labels.py Outdated Show resolved Hide resolved
tests/test_intersection_over_union.py Outdated Show resolved Hide resolved
jo-mueller and others added 5 commits November 9, 2021 11:02
@jo-mueller @haesleinhuepf
Update biapol_utilities/label/_label_overlap_matrix.py
701aa51 
Co-authored-by: Robert Haase <haesleinhuepf@users.noreply.github.com>
@jo-mueller @haesleinhuepf
Update biapol_utilities/label/_match_labels.py
159fd93 
Co-authored-by: Robert Haase <haesleinhuepf@users.noreply.github.com>
@jo-mueller
generalized IoU test
cd894ae
@jo-mueller
removed label_overlap_matrix
ad0e7f8 
The functionality is now directly implemented in the _intersection_over_union.py
@jo-mueller
Delete overlap_matrix_benchmarking.ipynb
56e718f
@jo-mueller
Copy link
Contributor Author

Synopsis:

  • Removed label_overlap_matrix (ad0e7f8)
  • generalized IoU test (cd894ae)
  • removed benchmark notebook (56e718f)

@haesleinhuepf
Copy link
Member

Hi Johannes @jo-mueller ,

this all looks good to me. Shall I merge?

@haesleinhuepf haesleinhuepf changed the title Compare overlap matrices Use scikit-learn confusion matrix instead of own custom code Nov 10, 2021
@jo-mueller
Copy link
Contributor Author

Yes :)

@haesleinhuepf haesleinhuepf merged commit c43da27 into main Nov 10, 2021
@haesleinhuepf haesleinhuepf deleted the compare_overlap_matrices branch November 10, 2021 19:03
@haesleinhuepf
Copy link
Member

Wonderful. Thanks for working on this @jo-mueller !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haesleinhuepf haesleinhuepf haesleinhuepf requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Check if scikit-learn has functions that we can reuse
2 participants
@jo-mueller @haesleinhuepf