marrlab / DomainLab

modular domain generalization: https://pypi.org/project/domainlab/
https://marrlab.github.io/DomainLab/
MIT License
40 stars 2 forks source link

do we need a property for trainer called decoratee (without ""_"") when self.model is always equal to decoratee? #825

Closed smilesun closed 4 days ago

smilesun commented 2 months ago

https://github.com/marrlab/DomainLab/blob/79a2524560f8a654ff4032dc992d35f4b7183027/domainlab/algos/trainers/a_trainer.py#L115

smilesun commented 2 months ago

as in the comment of abstract trainer stated, self.decorates can be both model or trainer but self._decoratee can only be trainer, self.get_model() can only be model

smilesun commented 2 months ago

after the change below, now self.model can be both trainer and model

<!DOCTYPE html> Name  

Latest commitsmilesun2 minutes agoHistoryMerge pull request #823 from marrlab/trainer_as_model4970eb6treat trainer as model

Name
Latest commit smilesun smilesun 2 minutes ago History

Merge pull request https://github.com/marrlab/DomainLab/pull/823 from marrlab/trainer_as_model

4970eb6

treat trainer as model

MatteoWohlrapp commented 4 weeks ago

I don't think we need it. Decoratee acts as an abstraction to check if _decoratee is none, and if so accesses the model instead:

@property def decoratee(self): if self._decoratee is None: return self.model return self._decoratee

However, we are already assigning _decoratee to model in the constructor and could therefore also directly call self.model instead:

if self._decoratee is not None: self._decoratee.init_business( model, task, observer, device, aconf, flag_accept ) self.model = self._decoratee else: self.model = model

I don't think this is particularly intuitive because model can then also be a trainer. We could leave the decoratee but rename _decoratee to trainer. That way decoratee could make the distinction between model and trainer and we only access decoratee.