-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added MACL(Model Aware Contrastive learning ) Loss Function #1757
base: master
Are you sure you want to change the base?
Conversation
Added MACL loss Function in Loss folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution and sorry for the late review!
Could you add a minimal test in tests/loss/test_macl_loss.py
? It is enough to pass some dummy values and check that the loss returns a valid value.
Could you also import the loss in lightly/loss/__init__.py
? See https://github.com/lightly-ai/lightly/blob/master/lightly/loss/__init__.py, then it can be directly accessed as form lightly.loss import MACLLoss
def __init__(self, temperature=0.1, alpha=0.5, A_0=0): | ||
super().__init__() | ||
self.t_0 = temperature | ||
self.alpha = alpha | ||
self.A_0 = A_0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def __init__(self, temperature=0.1, alpha=0.5, A_0=0): | |
super().__init__() | |
self.t_0 = temperature | |
self.alpha = alpha | |
self.A_0 = A_0 | |
def __init__(self, temperature: float = 0.1, alpha: float = 0.5, A_0: float = 0): | |
super().__init__() | |
self.temperature = temperature | |
self.alpha = alpha | |
self.A_0 = A_0 |
Could you also add a short docstring that describes what the loss does, what the parameters are, and add a reference to the paper that introduced the loss? You can follow the example here:
lightly/lightly/loss/ntx_ent_loss.py
Line 17 in 356ae56
class NTXentLoss(nn.Module): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok i will add the loss doc also
Co-authored-by: guarin <[email protected]>
Added MACL(Model Aware Contrastive learning ) Loss Function in Loss folder