Deep-MI / FastSurfer

PyTorch implementation of FastSurferCNN
Apache License 2.0
456 stars 119 forks source link

conform.py bug #505

Closed neurolabusc closed 5 months ago

neurolabusc commented 5 months ago

Description

The conform.py has a bug, which will impact behavior whenever the parameter f_low is not zero. Fortunately, zero is the default for f_low. Fixing this would allow sophisticated usage and development.

The issue is that src_min is initially set to the global minimum. However, f_low is subsequently updated to the lower output threshold. The problem is that final computation of the output high threshold (src_max) assumes the value of src_min is still the global minimum:

    src_min = idx * bin_size + src_min

    # get upper limit
    nth = voxnum - int((1.0 - f_high) * nz)
    idx = np.where(cs >= nth)

    if len(idx[0]) > 0:
        idx = idx[0][0] - 2

    else:
        print("ERROR: rescale upper bound not found")

    src_max = idx * bin_size + src_min

An easy fix would be to explicitly create and refer to a global_min variable that is independent from the output minimum:

    global_min = src_min
    src_min = idx * bin_size + global_min

    # get upper limit
    nth = voxnum - int((1.0 - f_high) * nz)
    idx = np.where(cs >= nth)

    if len(idx[0]) > 0:
        idx = idx[0][0] - 2

    else:
        print("ERROR: rescale upper bound not found")

    src_max = idx * bin_size + global_min
dkuegler commented 5 months ago

I think I have just confirmed the existence of this bug. When we fix this, we might as well make the math easier to understand though.

dkuegler commented 5 months ago

This has been fixed by #506.