Skip to content

Should AsDiscrete be refactored in favour of clarity and generality? #3371

@mattwarkentin

Description

@mattwarkentin

Discussed in #3346

Originally posted by mattwarkentin November 15, 2021
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?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions