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

Expose iter_size solver option #744

Merged
merged 2 commits into from
May 18, 2016
Merged

Conversation

lukeyeager
Copy link
Member

@lukeyeager lukeyeager commented May 16, 2016

Close #660
Replace #675

Expose iter_size solver option (BVLC/caffe#1977) for NVcaffe >= 0.14.

/cc @drozdvadym

$('#iter-size-option').show();
}
{% endif %}
{% endfor %}
Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe you can extend the structure on lines 414-416 with a can_accumulate_gradients field for a more direct access to framework properties?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah, good call. I also forgot to add this to the Generic path ...

network = network,
random_seed = form.random_seed.data,
solver_type = form.solver_type.data,
shuffle = form.shuffle.data,
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the whitespace changes. It had to be done at some point.

@lukeyeager
Copy link
Member Author

Refactored to use the name batch_accumulation instead of iter_size.

@gheinrich
Copy link
Contributor

That looks good to me - I haven't tested let me know if you want me to.

@lukeyeager
Copy link
Member Author

Actually it would be great if you could do a quick smoketest. No need to validate convergence, just check that:

  1. The option doesn't appear for Torch
  2. It does appear for NVcaffe >= 0.14
  3. When you set the value to 2, the generated solver.prototxt contains the new value

Assuming all is good, please merge. Thanks!

from digits.frameworks import CaffeFramework
if CaffeFramework().can_accumulate_gradients():
solver.iter_size = self.batch_accumulation

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to make this change in save_files_generic() too.

Copy link
Member Author

@lukeyeager lukeyeager May 18, 2016

Choose a reason for hiding this comment

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

d'oh. Done.

@gheinrich
Copy link
Contributor

The classification path is working for me - there is a little omission in the generic path.

Rename iter_size -> batch_accumulation (less Caffe-specific)
Disable for Torch and older versions of Caffe
@lukeyeager lukeyeager merged commit 03e7f11 into NVIDIA:master May 18, 2016
@lukeyeager lukeyeager deleted the iter_size branch May 18, 2016 16:51
@lukeyeager lukeyeager mentioned this pull request May 18, 2016
6 tasks
SlipknotTN pushed a commit to cynnyx/DIGITS that referenced this pull request Mar 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants