Should AsDiscrete be refactored in favour of clarity and generality?

[mattwarkentin]

Hi,

For the AsDiscrete/AsDiscreted transform - I find the logit_thresh argument to be confusing. I have also found it confusing to have separate arguments for whether something happens (threshold_values) and how it happens(logit_thresh). Also, the default threshold value is 0.5, which suggests to me that we are thresholding on a probability scale, despite the name of the argument (a logit of 0.5 corresponds to a probability of 0.6225, which seems odd, if logit was intended).

I am wondering if this transform should be simplified to be more clear and made more general/agnostic. Something like:

AsDiscrete(argmax=False, onehot=None, threshold=None, round=None)

Args:
    argmax: whether to execute argmax function on input data before transform.
        Defaults to ``False``.
    onehot: If not ``None``, the number of classes to convert to for one-hot encoding.
        Defaults to ``None``.
    threshold: If not ``None``, the threshold value for thresholding operation.
        Defaults to ``None``.
    round: if not ``None``, round the data according to the specified option,
        available options: ["torchrounding"].

This way, the transform is agnostic toward the input data scale (logit/prob/other), and the user controls whether an operation occurs with a single argument, rather than two. I also think the documentation could be improved to make it clear that if multiple operations are requested that they are applied sequentially in the order of the arguments. What do you think?

[Nic-Ma]

Hi @mattwarkentin ,

Thanks for your feedback, let me try to make it clear ASAP.