-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Layers declare their types and number of bottom/top blobs #479
Layers declare their types and number of bottom/top blobs #479
Conversation
Nice unification.
Since the blob check should be done for every Thoughts? |
Yeah, I'd also thought about having each layer make the call to the parent explicitly, but went this way because (as you said) it's more automatic. But I think you're right, it's safer not to have to remember to make an additional call everywhere SetUp is called, so I'll do it that way. Thanks. |
Re-implemented as suggested (with base Layer::SetUp calling CheckBlobCounts and each Layer subclass explicitly calling Layer::SetUp). Please take another look when you get a chance, @shelhamer. |
check top/bottom blob counts explicitly in SetUp; instead call base Layer::SetUp.
Once again, nice tidying up. Thanks Jeff! |
…lobs Layers declare their types and number of bottom/top blobs
virtual inline LayerParameter_LayerType type() const { | ||
return LayerParameter_LayerType_ARGMAX; | ||
} | ||
virtual inline int MinBottomBlobs() const { return 1; } |
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.
@jeffdonahue I think argmax need to have one bottom blob and can have at at least 1 top blob and at most 2
Thanks for noticing that @sguada. I pushed the fix for the bottom blob count directly to dev (see below), but it seems to only support 1 top blob currently (and was previously checking as such) so I left that as is.
|
…numblobs Layers declare their types and number of bottom/top blobs
This PR attempts to unify/standardize all the "___ Layer takes N blobs as input." messages at the top of each layer's SetUp method throughout the code. Adds 6 methods:
ExactNumBottomBlobs
,MinBottomBlobs
,MaxBottomBlobs
, and the analogs forTop
, which should be overridden by layers requiring a specific min/max/exact number of bottom/top blobs (which currently is all of them, as far as I know).The design I settled on was to actually perform these checks in an additional method,
CheckBlobCounts
, whichNet::Init
now calls right beforeSetUp
. The drawback is that now anySetUp
call is not protected by a blob count check. I also considered just changing theSetUp
method to be implemented only in theLayer
superclass and just have that method callCheckBlobCounts
andFurtherSetUp
, which would be the method that subclasses now override (as in the currentNeuronLayer
andLossLayer
s) -- do people think this would be a better design? (Or some other design entirely? Definitely open to suggestions.)