Implementation of the Modified Hausdorff Distance (MHD) metric

[robinthibaut]

Description

Greetings,

In the file skimage/metrics/set metric.py, I included the Modified Hausdorff Distance function. This brilliant library, I believe, would benefit greatly from the addition of this function. Dubuisson et al. [1] demonstrated that the Modified Hausdorff Distance (MHD) outperforms the directed Hausdorff Distance (HD).
I implemented unit tests and the benchmark for the new function in accordance with the contributing guidelines.

Best,
Robin

Reference
[1] M. P. Dubuisson and A. K. Jain. A Modified Hausdorff distance for object
matching. In ICPR94, pages A:566-568, Jerusalem, Israel, 1994.
http://ieeexplore.ieee.org/xpls/abs_all.jsp?arnumber=576361

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8
• Descriptive commit messages (see below)

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[pep8speaks]

Hello @robinthibaut! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file benchmarks/benchmark_metrics.py:

Line 29:80: E501 line too long (83 > 79 characters)

• In the file skimage/metrics/set_metrics.py:

Line 38:80: E501 line too long (83 > 79 characters)
Line 125:80: E501 line too long (83 > 79 characters)
Line 128:80: E501 line too long (83 > 79 characters)

• In the file skimage/metrics/tests/test_set_metrics.py:

Line 45:80: E501 line too long (80 > 79 characters)
Line 69:80: E501 line too long (80 > 79 characters)
Line 71:80: E501 line too long (80 > 79 characters)
Line 88:80: E501 line too long (80 > 79 characters)
Line 90:80: E501 line too long (80 > 79 characters)
Line 145:80: E501 line too long (87 > 79 characters)
Line 161:80: E501 line too long (80 > 79 characters)
Line 163:80: E501 line too long (80 > 79 characters)
Line 177:80: E501 line too long (80 > 79 characters)
Line 179:80: E501 line too long (83 > 79 characters)

Comment last updated at 2022-01-05 17:41:34 UTC

[grlee77]

This looks like a good addition and should be simple to maintain. The method seems well known (the conference publication has >1800 citations on google scholar).

The code itself looks good, my only comment was whether we should incorporate it as an option in the existing function to reduce duplication.

[grlee77]

Unless I am mistaken, the only difference with the existing function is the change from np.max to np.mean here.

Given, that I am wondering if it would be better to just add an optional modified keyword-only argument to the existing function instead? (defaulting to False for backwards compatibility). What do others think?

[robinthibaut]

You are correct, and I concur with your suggestion that adding a modified keyword to the original function would be more pythonic.
What course of action should we take? Should I combine the two functions as described above and commit the changes?

[grlee77]

@robinthibaut, sorry I had missed this question. Yes, please just add the kwarg and code path to the original function and then remove this new function.

A "Notes" section in the docstring can be used to describe the difference for the "modified" case.

[robinthibaut]

@grlee77 I made the changes, and I hope it is now satisfactory.

[grlee77]

Thanks @robinthibaut, this looks good to me now. I just made two small stylistic commits.

[rfezzani]

Thank you @robinthibaut, LGTM. I just left a style suggestion 😉

[robinthibaut]

Thank you @robinthibaut, LGTM. I just left a style suggestion 😉

Thank you @rfezzani ! I made the changes according to your suggestion 🙂

[rfezzani]

🎉 Merging! Thank you @robinthibaut 👏