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

some error in calc_IoU_sets function #10

Open
niuniupower opened this issue Jul 21, 2022 · 1 comment
Open

some error in calc_IoU_sets function #10

niuniupower opened this issue Jul 21, 2022 · 1 comment
Assignees
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@niuniupower
Copy link

def calc_IoU_Sets(truth, pred, c=1, **kwargs):
# Obtain sets with associated class
gt = np.equal(truth, c)
pd = np.equal(pred, c)
# Calculate IoU
if (pd.sum() + gt.sum() - np.logical_and(pd, gt).sum()) != 0:
iou = np.logical_and(pd, gt).sum() /
(pd.sum() + gt.sum() - np.logical_and(pd, gt).sum())
else : iou = 0.0
# Return computed IoU
return iou

In this function, you should reduce the FN, but you reduce the FP.

@muellerdo
Copy link
Member

muellerdo commented Jul 25, 2022

Hello @niuniupower,

mhm, I re-checked the correctness of the two implementations (set-wise and confusion-matrix-wise) of the IoU/Jaccard.
They should be fine.

Where exactly did you find an error?

For the formulas, I would refer to Wikipedia: https://en.wikipedia.org/wiki/Jaccard_index

Set-wise:
image

def calc_IoU_Sets(truth, pred, c=1, **kwargs):
    # Obtain sets with associated class
    gt = np.equal(truth, c)
    pd = np.equal(pred, c)
    # Calculate IoU
    if  (pd.sum() + gt.sum() - np.logical_and(pd, gt).sum()) != 0:
        iou = np.logical_and(pd, gt).sum() / \
              (pd.sum() + gt.sum() - np.logical_and(pd, gt).sum())
    else : iou = 0.0
    # Return computed IoU
    return iou

Confusion-matrix-wise:
image

def calc_IoU_CM(truth, pred, c=1, **kwargs):
    # Obtain confusion mat
    tp, tn, fp, fn = calc_ConfusionMatrix(truth, pred, c)
    # Calculate IoU
    if (tp + fp + fn) != 0 : iou = tp / (tp + fp + fn)
    else : iou = 0.0
    # Return computed IoU
    return iou

Cheers,
Dominik

UPDATE: Mhm, I see that there is a definition difference in the confusion-matrix-based formulas.

Tensorflow, Keras and key publications like Taha & Hanbury (https://bmcmedimaging.biomedcentral.com/track/pdf/10.1186/s12880-015-0068-x.pdf) define it with False-Negative instead of True-Negative as Wikipedia.

image

However, the reference in Wikipedia is also to the key paper of Taha & Hanbury, which indicates that we have found one of these rare mistakes in Wikipedia :o

UPDATE#2:
Updated the incorrect confusion-matrix based formula in the Wikipedia page for Jaccard: https://en.wikipedia.org/wiki/Jaccard_index

@muellerdo muellerdo added the invalid This doesn't seem right label Jul 25, 2022
@muellerdo muellerdo self-assigned this Jul 25, 2022
@muellerdo muellerdo added the bug Something isn't working label Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants