Allow BVLC/caffe and caffe-on-windows #769
Conversation
IsaacYangSLA
commented
May 22, 2016
IsaacYangSLA
commented
- parse_version may return ('00000001','_','00000003','_final'), which is hard to to convert back to '1.0.0-rc3.' Therefore, storing the original version string as a backup for Info menu.
- Flavor will be NVIDIA if libcaffe.so is built with NVIDIA enhancement (Linux only). In other cases, flavor is BVLC.
- Store information in CaffeOption class so no need to query caffe for linked libraries twice.
| """ | ||
| Errors that occur while performing tasks in unsupported platforms | ||
| """ | ||
| pass |
There was a problem hiding this comment.
[no newline at end document]
|
|
||
| class CaffeOption(config_option.FrameworkOption): | ||
| version_string = None | ||
| flavor = None |
There was a problem hiding this comment.
Let's not mix instance attributes and class attributes. wtforms does that and it's super confusing.
lukeyeager
changed the title
|
I've tested this and it lets me use either BVLC/caffe or NVIDIA/caffe in DIGITS - cool! |
|
I still have to keep Microsoft Caffe as one 'special case' because caffe.exe does not return version info currently. And linked library approach can't apply to Windows platform. Without such version info, validate_version raises exception. This [PR (https://github.com/BVLC/caffe/pull/4200)], which enables Windows Caffe to report version, was just created on BVLC/Windows branch. The same one was created on Microsoft's Caffe repository, but it looked like the PR should not go there. Therefore, that one was closed and I created the new one on BVLC. |
|
about parse_version may return ('00000001','','00000003','final'), i had it return with the same sting as on Linux, you may check bygreencn/caffe@38ff238 |
|
|
||
| if version is None: | ||
| if info_dict['ver_str'] is None: | ||
| raise config_option.BadValue('Could not get version information from caffe at "%s". Are you using the NVIDIA fork?' |
There was a problem hiding this comment.
Remove this comment about the NVIDIA fork - it's not required anymore
|
Modified the validate_version function. When no ver_str, raising an exception. Minimum version also depends on flavor of Caffe. |
|
Even though these checks currently work, they're misleading. Can you update them to only make the version check if the flavor is NVIDIA? BVLC/caffe > 1.0.0-rc3 (the minimum) supports all these features. https://github.com/NVIDIA/DIGITS/blob/c0862e1611/digits/frameworks/caffe_framework.py#L37 |
|
Tested and everything looks right: bvlc@rc2: fails |
|
Neat - I tried a parallel build on Travis for NVIDIA/caffe and BVLC/caffe and they both work great: @IsaacYangSLA please address #769 (comment) before merge |
|
Thanks. There is a commit immediately below that comment. It changed the behavior of 'validate_version' so it only checks minimum version on NVIDIA flavor. Sorry that I piggybacked it with removal of Windows version workaround. |
|
Now that BVLC/caffe#4200 is merged, this should work for the |
|
Code looks good and seems to be working well - thanks for the good work! Please squash and I'll merge. |
Use master or Windows branch from BVLC fork Parse caffe --version if available
Allow BVLC/caffe and caffe-on-windows
