Refactoring detection task

[sungchul2]

Summary

This PR includes:

• Create criterion modules for each model
• Enable Factory template for building model
• Remove dict type config -> in the next PR due to the conflicts w/ iseg

How to test

Checklist

• I have added unit tests to cover my changes.​
• I have added integration tests to cover my changes.​
• I have ran e2e tests and there is no issues.
• I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
• I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
• I have linked related issues.

License

• I submit my code changes under the same Apache License that covers the project.
  Feel free to contact the maintainers if that's a concern.
• I have updated the license header for each file (see an example below).

# Copyright (C) 2024 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

[sungchul2]

@kprokofi @harimkang We changed modules' names that are affected by factory class to ~Module such as SSDHeadModule (previously NNSSDHead), but I found the below part that uses a module's name to internally patch a variable.
I'm not sure if similar parts are there more, but I think changing the module's name doesn't seem like an appropriate decision because we need to change all parts that are affected by our new decision.
I'd like to suggest setting factory's name such as SSDHeadBuilder instead.

training_extensions/src/otx/core/model/detection.py

Line 468 in 71a819e

background_class = hasattr(self.model, "bbox_head") and isinstance(self.model.bbox_head, SSDHead)

[eunwoosh]

@kprokofi @harimkang We changed modules' names that are affected by factory class to ~Module such as SSDHeadModule (previously NNSSDHead), but I found the below part that uses a module's name to internally patch a variable. I'm not sure if similar parts are there more, but I think changing the module's name doesn't seem like an appropriate decision because we need to change all parts that are affected by our new decision. I'd like to suggest setting factory's name such as SSDHeadBuilder instead.

training_extensions/src/otx/core/model/detection.py

Line 468 in 71a819e

background_class = hasattr(self.model, "bbox_head") and isinstance(self.model.bbox_head, SSDHead)

Honestly, I think it's better to make factory function than factory class. Recently, factory class with __new__ implementation is often used, but it looks weird that output of SomeClass(...) isn't instance of SomeClass. If class is preferred, than I think using class method like SSDHeadBuilder.make is better. Anyway it looks good to decide a policy of factory class for model at this point.

[sungchul2]

Honestly, I think it's better to make factory function than factory class. Recently, factory class with __new__ implementation is often used, but it looks weird that output of SomeClass(...) isn't instance of SomeClass. If class is preferred, than I think using class method like SSDHeadBuilder.make is better. Anyway it looks good to decide a policy of factory class for model at this point.

Yes, I think it's a problem that output of SomeClass(...) isn't instance of SomeClass as @eunwoosh said.
Using factory function can be alternative, but I think it is important to maintain module's name no matter what method we choose.
If we use factory class, we can change factory's name like below:

class SSDHead: ...

class SSDHeadBuilder:
    SSDHEAD_CFG: ClassVar[dict[str, Any]] = {...}
    def __new__(cls, version: str, ...):
        ...
        return SSDHead(**SSDHEAD_CFG[version], ...)

For factory function, we can use below:

SSDHEAD_CFG: dict[str, Any] = {...}
def ssd_head_builder(version: str, ...):
    ...
    return SSDHead(**SSDHEAD_CFG[version], ...)

[eunwoosh]

Thanks for your work :)
I agree overall direction of this PR, I have a one thing to discuss.
I'm not sure that we should change every module like a factory class even if that module supports single case. IMHO, it's better to leave it as it is and change it later if that module needs to support multiple cases.
How do you think about that?

[sungchul2]

Thanks for your work :) I agree overall direction of this PR, I have a one thing to discuss. I'm not sure that we should change every module like a factory class even if that module supports single case. IMHO, it's better to leave it as it is and change it later if that module needs to support multiple cases. How do you think about that?

Yeah, it seems unnecessary but I think it must exist for scalability and unified structure.