Refactor/evaluation task #22
Conversation
quadra/configs/experiment/base/segmentation/smp_multiclass_evaluation.yaml
Outdated
Show resolved
Hide resolved
quadra/tasks/classification.py
Outdated
| super().prepare() | ||
| self.deployment_model = self.model_path |
There was a problem hiding this comment.
Why this is not done in the super? We need to manually assign everytime or I'm missing something
There was a problem hiding this comment.
Ok I saw it may be different for plain torch import
There was a problem hiding this comment.
I would probably still set it in super and override the prepare entirely probably, otherwise you need to remember this every time
There was a problem hiding this comment.
Actually the only "problem" exists for patchsklearn and sklearnclassif. which don't have a deployment model but backbone + classifier. To move the self.deployment_model = self.model_path in Evaluation i will overwrite the deployment model setter and set classifier and backbone in there. Without really setting a deployment_model object, which is not used in patchsklearn and sklearnclassification.
There was a problem hiding this comment.
Done, see if you like that. Commit: "style: Move self.deployment_model = self.model_path to parent class"
p.s. i know it's not "style", "refactor" would have been better
|
Amazing job @AlessandroPolidori! |
quadra/tasks/classification.py
Outdated
|
|
||
| @deployment_model.setter | ||
| def deployment_model(self, model_path: str): | ||
| """Set backbone and classifier.""" | ||
| try: | ||
| self.backbone = OmegaConf.load( |
There was a problem hiding this comment.
I don't get it, why should we be forced to use pytorch here?
I think we should support any kind of model as per other tasks, if torch is not used we don't generate gradients.
Or am I missing something?
There was a problem hiding this comment.
See commit "fix: Model import in sklearn evaluations now supports .pt and .pth model files" and tell me what you think
| return None | ||
|
|
||
| @deployment_model.setter | ||
| def deployment_model(self, model_path: str): |
There was a problem hiding this comment.
Same here, I would probably expect that we can deploy the model without a backbone, but if a deployed model is present it should be the first option
lorenzomammana
assigned lorenzomammana and AlessandroPolidori and unassigned lorenzomammana
|
Can we also have a short tutorial starting with training a model and running |
Summary
Describe the purpose of the pull request, including:
Only SegmentationEvaluation extends Evaluation. Also, we need to uniform the evaluation tasks as much as possible.
Currently they are unnecessarily different (attributes names are different, some use experiment_path while others the model_path).
Also SegmentationEvaluation was not tested, i added two tests (one for multiclass, one for binary).
How does it solve the problem?
All the children evaluations extend the Evaluation class.
What are the benefits of the solution?
Clean code.
Reference any relevant issues or pull requests.
Evaluation Task not used #21
Type of Change
Please select the one relevant option below:
Checklist
Please confirm that the following tasks have been completed: