Project-MONAI / MONAI

AI Toolkit for Healthcare Imaging
https://monai.io/
Apache License 2.0
5.68k stars 1.04k forks source link

Revisit "include_background" variable name in loss functions #7056

Open ericspod opened 11 months ago

ericspod commented 11 months ago

I believe that using "include_channel_zero" as an alternative to "include_background" may not be the best choice due to the loss of argument semantics. In contrast, the name "include_background" directly conveys the purpose of this argument to the user. The core issue of original post is that the channel of background is confusing. One potential solution could be to introduce an optional argument, such as background_channel: int | Sequence[int] = 0, to clarify this aspect. BTW, argument such as include_channel: int | Sequence[int] = 0 could be better than "include_channel_zero" .

Originally posted by @ChenglongWang in https://github.com/Project-MONAI/MONAI/discussions/6915#discussioncomment-7097583

See whole discussion for further feedback.

myron commented 11 months ago

I actually like "include_background", and I'd vote to keep it as is, simply because we've been using it for a long time now. And in the class doc it's explained what it means, if anyone is confused, it's really easy to read the doc (or comment in the class .py) . I've never encountered situations when the background (if present) is at a different non-zero channel. Unless there is a real use-case, I don't think we should over-complicate the api.