SforAiDl / vformer

A modular PyTorch library for vision transformer models
https://vformer.readthedocs.io/
MIT License
162 stars 22 forks source link

Remove _Projection class #81

Closed abhi-glitchhg closed 2 years ago

abhi-glitchhg commented 2 years ago

We can replace _Projection class with a one-liner if-else statement.

Should we replace it with if-else or should we keep the current implementation?

cc: @NeelayS @aditya-agrawal-30502 @alvanli

codecov-commenter commented 2 years ago

Codecov Report

Merging #81 (a7637d9) into main (aec005b) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #81   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        63           
  Lines         1880      1872    -8     
=========================================
- Hits          1880      1872    -8     
Impacted Files Coverage Δ
vformer/attention/cross.py 100.00% <100.00%> (ø)
NeelayS commented 2 years ago

@aditya-agrawal-30502 You had worked on this implementation, right? Were there any reasons behind making it a class?

aditya-agrawal-30502 commented 2 years ago

It was just that i needed a linear projection and had to use it a couple times so made a class of it. There was no implementation of it before so hence needed

NeelayS commented 2 years ago

I see. Since it's only a couple of times, we can remove it I guess. What you think, @abhi-glitchhg?

abhi-glitchhg commented 2 years ago

I see. Since it's only a couple of times, we can remove it I guess. What you think, @abhi-glitchhg?

yes,

NeelayS commented 2 years ago

Cool, merging.