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

Hotfix for accuracy interfering with training #5987

Closed
wants to merge 1 commit into from

Conversation

Noiredd
Copy link
Member

@Noiredd Noiredd commented Oct 16, 2017

Instead of reusing "unused" memory of the bottom blob (diff), it's safer to allocate some internal blob (gpu_buffer_) to make sure the layer does not interact with external memory that might be potentially used elsewhere (as was demonstrated in #5981). Bug was initially introduced in 62e0c85.

In the long run, we might want to optimize this to eg. only allocate GPU memory (instead of an entire blob), or even take a closer look at what is exactly going on in that buffer (i.e. how much memory do we actually need). On the other hand, using Accuracy layer during training is not very common usage - so we might leave this PR pending for now (for the affected users to pull) and only accept after said optimizations.

@Noiredd
Copy link
Member Author

Noiredd commented Oct 16, 2017

@shaibagon Can we optimize this memory-wise in any way? I.e. do we need the intermediate blob to be as large as the whole bottom[0]? If not, I say we merge. @shelhamer?

@shelhamer shelhamer added the bug label Oct 16, 2017
@ghost
Copy link

ghost commented Nov 12, 2017

Hi,
what the progress on merging this fix? It is already almost a month for caffe not being able to calculate/display accuracy statistics in training phase.

@shelhamer
Copy link
Member

@Noiredd see #6202 for a combined fix to the scratch usage of bottoms diffs. I decided to clear the diffs in that fix instead of needing separate memory. Please let me know what you think of that choice.

@Noiredd Noiredd deleted the accuracy-hotfix branch February 12, 2018 16:48
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.

2 participants