add CompactConvolutionalTransformer (CCT) architecture #272
Draft
felixdittrich92 wants to merge 2 commits intofrgfm:mainfrom
Draft
add CompactConvolutionalTransformer (CCT) architecture #272felixdittrich92 wants to merge 2 commits intofrgfm:mainfrom
felixdittrich92 wants to merge 2 commits intofrgfm:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Author
|
/quack review |
![darth-quack[bot]](https://avatars.githubusercontent.com/in/319938?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the PR 🙏
Other sections
holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L38holocron/models/classification/cct.py:L93holocron/models/classification/cct.py:L163holocron/models/classification/cct.py:L173holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L209holocron/models/classification/cct.py:L237holocron/models/classification/cct.py:L262holocron/models/classification/cct.py:L287holocron/models/classification/cct.py:L312holocron/models/classification/cct.py:L57holocron/models/classification/cct.py:L84holocron/models/classification/cct.py:L88holocron/models/classification/cct.py:L149holocron/models/classification/cct.py:L153holocron/models/classification/cct.py:L169holocron/models/classification/cct.py:L80holocron/models/classification/cct.py:L83holocron/models/classification/cct.py:L88holocron/nn/modules/transformers.py:L15holocron/nn/modules/transformers.py:L38holocron/nn/modules/transformers.py:L75holocron/nn/modules/transformers.py:L97holocron/nn/modules/transformers.py:L30holocron/nn/modules/transformers.py:L57holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L136For a better experience, it's recommended to check the review comments in the tab Files Changed.
Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment
| ) | ||
|
|
||
|
|
||
| class CCT_2_Checkpoint(Enum): |
There was a problem hiding this comment.
Let's use the regular PascalCase convention for classes
Pattern explanation 👈
For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# class moduleParser:
class ModuleParser:Quack feedback loop 👍👎
This comment is about [class-naming]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
|
|
||
| class ConvPatchEmbed(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.kaiming_normal_(m.weight) | ||
|
|
||
|
|
||
| class CCT(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.constant_(m.weight, 1.0) | ||
|
|
||
|
|
||
| def _cct( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| return _configure_model(model, checkpoint, progress=progress) | ||
|
|
||
|
|
||
| def _checkpoint( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| mask (torch.Tensor): optional mask | ||
| """ | ||
|
|
||
| scores = torch.matmul(query, key.transpose(-2, -1)) / math.sqrt(query.size(-1)) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| self.linear_layers = nn.ModuleList([nn.Linear(d_model, d_model) for _ in range(3)]) | ||
| self.output_linear = nn.Linear(d_model, d_model) | ||
|
|
||
| def forward(self, query: torch.Tensor, key: torch.Tensor, value: torch.Tensor, mask=None) -> torch.Tensor: |
There was a problem hiding this comment.
Would you mind adding type annotations? 🙏
Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def is_upper(variable_name):
def is_upper(variable_name: str) -> bool:Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
|
|
||
| def forward(self, x: torch.Tensor, mask: Optional[torch.Tensor] = None) -> torch.Tensor: | ||
|
|
||
| output = x |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
There was a problem hiding this comment.
Thanks for the PR 🙏
Other sections
holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L38holocron/models/classification/cct.py:L93holocron/models/classification/cct.py:L163holocron/models/classification/cct.py:L173holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L209holocron/models/classification/cct.py:L237holocron/models/classification/cct.py:L262holocron/models/classification/cct.py:L287holocron/models/classification/cct.py:L312holocron/models/classification/cct.py:L57holocron/models/classification/cct.py:L84holocron/models/classification/cct.py:L88holocron/models/classification/cct.py:L149holocron/models/classification/cct.py:L153holocron/models/classification/cct.py:L169holocron/models/classification/cct.py:L80holocron/models/classification/cct.py:L83holocron/models/classification/cct.py:L88holocron/nn/modules/transformers.py:L15holocron/nn/modules/transformers.py:L38holocron/nn/modules/transformers.py:L75holocron/nn/modules/transformers.py:L97holocron/nn/modules/transformers.py:L30holocron/nn/modules/transformers.py:L57holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L136For a better experience, it's recommended to check the review comments in the tab Files Changed.
Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment
| ) | ||
|
|
||
|
|
||
| class CCT_2_Checkpoint(Enum): |
There was a problem hiding this comment.
Let's use the regular PascalCase convention for classes
Pattern explanation 👈
For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# class moduleParser:
class ModuleParser:Quack feedback loop 👍👎
This comment is about [class-naming]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
|
|
||
| class ConvPatchEmbed(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.kaiming_normal_(m.weight) | ||
|
|
||
|
|
||
| class CCT(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.constant_(m.weight, 1.0) | ||
|
|
||
|
|
||
| def _cct( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| return _configure_model(model, checkpoint, progress=progress) | ||
|
|
||
|
|
||
| def _checkpoint( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| mask (torch.Tensor): optional mask | ||
| """ | ||
|
|
||
| scores = torch.matmul(query, key.transpose(-2, -1)) / math.sqrt(query.size(-1)) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| self.linear_layers = nn.ModuleList([nn.Linear(d_model, d_model) for _ in range(3)]) | ||
| self.output_linear = nn.Linear(d_model, d_model) | ||
|
|
||
| def forward(self, query: torch.Tensor, key: torch.Tensor, value: torch.Tensor, mask=None) -> torch.Tensor: |
There was a problem hiding this comment.
Would you mind adding type annotations? 🙏
Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def is_upper(variable_name):
def is_upper(variable_name: str) -> bool:Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
|
|
||
| def forward(self, x: torch.Tensor, mask: Optional[torch.Tensor] = None) -> torch.Tensor: | ||
|
|
||
| output = x |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
There was a problem hiding this comment.
Thanks for the PR 🙏
Other sections
holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L38holocron/models/classification/cct.py:L93holocron/models/classification/cct.py:L163holocron/models/classification/cct.py:L173holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L209holocron/models/classification/cct.py:L237holocron/models/classification/cct.py:L262holocron/models/classification/cct.py:L287holocron/models/classification/cct.py:L312holocron/models/classification/cct.py:L57holocron/models/classification/cct.py:L84holocron/models/classification/cct.py:L88holocron/models/classification/cct.py:L149holocron/models/classification/cct.py:L153holocron/models/classification/cct.py:L169holocron/models/classification/cct.py:L80holocron/models/classification/cct.py:L83holocron/models/classification/cct.py:L88holocron/nn/modules/transformers.py:L15holocron/nn/modules/transformers.py:L38holocron/nn/modules/transformers.py:L75holocron/nn/modules/transformers.py:L97holocron/nn/modules/transformers.py:L30holocron/nn/modules/transformers.py:L57holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L136For a better experience, it's recommended to check the review comments in the tab Files Changed.
Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment
| ) | ||
|
|
||
|
|
||
| class CCT_2_Checkpoint(Enum): |
There was a problem hiding this comment.
Let's use the regular PascalCase convention for classes
Pattern explanation 👈
For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# class moduleParser:
class ModuleParser:Quack feedback loop 👍👎
This comment is about [class-naming]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
|
|
||
| class ConvPatchEmbed(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.kaiming_normal_(m.weight) | ||
|
|
||
|
|
||
| class CCT(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.constant_(m.weight, 1.0) | ||
|
|
||
|
|
||
| def _cct( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| return _configure_model(model, checkpoint, progress=progress) | ||
|
|
||
|
|
||
| def _checkpoint( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| mask (torch.Tensor): optional mask | ||
| """ | ||
|
|
||
| scores = torch.matmul(query, key.transpose(-2, -1)) / math.sqrt(query.size(-1)) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| self.linear_layers = nn.ModuleList([nn.Linear(d_model, d_model) for _ in range(3)]) | ||
| self.output_linear = nn.Linear(d_model, d_model) | ||
|
|
||
| def forward(self, query: torch.Tensor, key: torch.Tensor, value: torch.Tensor, mask=None) -> torch.Tensor: |
There was a problem hiding this comment.
Would you mind adding type annotations? 🙏
Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def is_upper(variable_name):
def is_upper(variable_name: str) -> bool:Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
|
|
||
| def forward(self, x: torch.Tensor, mask: Optional[torch.Tensor] = None) -> torch.Tensor: | ||
|
|
||
| output = x |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
There was a problem hiding this comment.
Thanks for the PR 🙏
Other sections
holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L38holocron/models/classification/cct.py:L93holocron/models/classification/cct.py:L163holocron/models/classification/cct.py:L173holocron/models/classification/cct.py:L189holocron/models/classification/cct.py:L209holocron/models/classification/cct.py:L237holocron/models/classification/cct.py:L262holocron/models/classification/cct.py:L287holocron/models/classification/cct.py:L312holocron/models/classification/cct.py:L57holocron/models/classification/cct.py:L84holocron/models/classification/cct.py:L88holocron/models/classification/cct.py:L149holocron/models/classification/cct.py:L153holocron/models/classification/cct.py:L169holocron/models/classification/cct.py:L80holocron/models/classification/cct.py:L83holocron/models/classification/cct.py:L88holocron/nn/modules/transformers.py:L15holocron/nn/modules/transformers.py:L38holocron/nn/modules/transformers.py:L75holocron/nn/modules/transformers.py:L97holocron/nn/modules/transformers.py:L30holocron/nn/modules/transformers.py:L57holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L67holocron/nn/modules/transformers.py:L136For a better experience, it's recommended to check the review comments in the tab Files Changed.
Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment
| ) | ||
|
|
||
|
|
||
| class CCT_2_Checkpoint(Enum): |
There was a problem hiding this comment.
Let's use the regular PascalCase convention for classes
Pattern explanation 👈
For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# class moduleParser:
class ModuleParser:Quack feedback loop 👍👎
This comment is about [class-naming]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
|
|
||
| class ConvPatchEmbed(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.kaiming_normal_(m.weight) | ||
|
|
||
|
|
||
| class CCT(nn.Module): |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| nn.init.constant_(m.weight, 1.0) | ||
|
|
||
|
|
||
| def _cct( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| return _configure_model(model, checkpoint, progress=progress) | ||
|
|
||
|
|
||
| def _checkpoint( |
There was a problem hiding this comment.
This section seems to be untested, let's fix that together!
Pattern explanation 👈
As your program evolves, refactoring and modifications may subtly cause regressions. Adding well-designed tests for your features will help spot them easily and consistently. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
import pytest
from my_lib.submodule import MyClass
@pytest.mark.parametrize(
"input_arg, expected_output",
[
[YOUR_INPUT, YOUR_OUTPUT],
],)
def test_myclass_custommethod(input_arg, expected_output):
obj = MyClass()
assert obj.custommethod(input_arg) == expected_outputQuack feedback loop 👍👎
This comment is about [untested]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| mask (torch.Tensor): optional mask | ||
| """ | ||
|
|
||
| scores = torch.matmul(query, key.transpose(-2, -1)) / math.sqrt(query.size(-1)) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| self.linear_layers = nn.ModuleList([nn.Linear(d_model, d_model) for _ in range(3)]) | ||
| self.output_linear = nn.Linear(d_model, d_model) | ||
|
|
||
| def forward(self, query: torch.Tensor, key: torch.Tensor, value: torch.Tensor, mask=None) -> torch.Tensor: |
There was a problem hiding this comment.
Would you mind adding type annotations? 🙏
Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def is_upper(variable_name):
def is_upper(variable_name: str) -> bool:Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
| ] | ||
|
|
||
| # apply attention on all the projected vectors in batch | ||
| x, attn = scaled_dot_product_attention(query, key, value, mask=mask) |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
|
|
||
| def forward(self, x: torch.Tensor, mask: Optional[torch.Tensor] = None) -> torch.Tensor: | ||
|
|
||
| output = x |
There was a problem hiding this comment.
It doesn't look like we need this intermediate variable
Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.Code example
Here is an example of how this is typically handled:
# def make_upper(input_str: str) -> str:
# upper_str = input_str.upper()
# return upper_str
def make_upper(input_str: str) -> str:
return input_str.upper()Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
nn.TransformerEncoderwould work also but is still not onnx exportable)NOTE:
@frgfm as discussed there is some space for improvements (have fun 😄)
(cct_2.pth attached)
cct_2.zip