-
Notifications
You must be signed in to change notification settings - Fork 323
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 swin swav model #975
base: master
Are you sure you want to change the base?
added swin swav model #975
Conversation
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.
can we get some tests for this addition?
afdc09a
to
c5137b4
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
yeah sure, working on it. |
…tning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
@AhmedHamdi101 how is it going here? 🐰 |
I am working on it, I will be submitting an update soon this week 😃 |
for more information, see https://pre-commit.ci
…101/lightning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
modified torchvision functions to be based on the library version.
…101/lightning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
…tning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
…tning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
Modified the MLP, Premute, and StochasticDepth definition
…tning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
|
||
if not hasattr(torchvision.ops.misc, "MLP"): | ||
|
||
class MLP(torch.nn.Sequential): |
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.
let's rather create a wrapper on init
which checks the compatibility, this declaration under if is tricky as a user may have it or not hard to guess if because of old Bolts or other dependency
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.
@Borda can you elaborate more and give me a short example of what you mean 😅
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.
we could use #1056 similar as Flas does but much simpler
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.
@Borda I have used the function requires
you mentioned but I modified the if condition in line 13-16
by adding a not
reqs = [ ModuleAvailableCache(mod_ver) if "." **not** in mod_ver else RequirementCache(mod_ver) for mod_ver in module_path_version ]
As I think there was a bug in it, please check it out.
…tning-bolts into feature/add_swav_swin
to check on torchvision version
for more information, see https://pre-commit.ci
by adding a not in the if condition of reqs list
'feature/add_swav_swin' of github.com:AhmedHamdi101/lightning-bolts into feature/add_swav_swin
for more information, see https://pre-commit.ci
@Borda any updates? 😅 |
What does this PR do?
Resolves #974
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Yea !