Skip to content
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

small bugs and small improvements #3913

Merged
merged 3 commits into from
Apr 14, 2017
Merged

small bugs and small improvements #3913

merged 3 commits into from
Apr 14, 2017

Conversation

antran89
Copy link
Contributor

@antran89 antran89 commented Mar 30, 2016

This PR addresses small improvements in Caffe codes such as small programming bugs, typos and etc.

  • There is a small bug in pooling_layer.cu line 141. The cumsum should be initialized as 0. for cumulative sum. I have not tested it, but logically, it should be initialized with 0..
  • Improvements in compute_image_mean.cpp program: Print output to stderr (while still logging). It was tested on my LDMB database.

@seanbell
Copy link

@Yangqing You were the last to edit this file (c8e7cce) -- why did you change it to FLT_MIN?

@seanbell
Copy link

@antran89 looking at this function, it seems that the last line of the kernel should also be updated to:
top_data[index] = (cumsum > 0.) ? (cumvalues / cumsum) : 0.; to avoid divide-by-zero.

@Yangqing
Copy link
Member

@seanbell Well, c8e7cce was a bug :)

@seanbell seanbell added the bug label Apr 1, 2016
@seanbell
Copy link

seanbell commented Apr 1, 2016

@Yangqing no worries -- I was just checking to make sure, thanks :)

@@ -149,7 +149,7 @@ __global__ void StoPoolForwardTest(const int nthreads,
cumvalues += bottom_slice[h * width + w] * bottom_slice[h * width + w];
}
}
top_data[index] = cumvalues / cumsum;
top_data[index] = (cumsum > 0.) ? cumvalues / cumsum : 0.;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the test for cumsum > 0. correct here. Do we not allow negative values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elezar You are right? It should be cumsum != 0. for safe.
@seanbell What do you think?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negative values aren't valid for stochastic pooling. The input represents values in a probability distribution (though it doesn't have to be normalized to sum to 1).

@antran89 antran89 changed the title small bug in pooling_layer.cu small bugs and small improvements Aug 15, 2016
@shelhamer shelhamer merged commit c11e782 into BVLC:master Apr 14, 2017
beniz pushed a commit to jolibrain/caffe that referenced this pull request Jul 10, 2017
minor fix to stochastic pooling and clean-up of mean computation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants