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

Add option to use multiple pretrained networks #498

Merged
merged 6 commits into from
Jan 18, 2016

Conversation

IgorX2
Copy link
Contributor

@IgorX2 IgorX2 commented Jan 8, 2016

I've added an option to copy weights from multiple pretrained networks so that ensembles can be built easily. Multiple pretrained networks can be added as semicolon delimited paths.

@lukeyeager
Copy link
Member

That's a neat idea. Can you link to a blog post or walkthrough that would let me test this out?

  1. You should update the tooltip to let users know about this feature
  2. Is there any way to add a test for this?

@lukeyeager
Copy link
Member

@gheinrich is this feasible for Torch as well?

@IgorX2
Copy link
Contributor Author

IgorX2 commented Jan 8, 2016

I needed this a few days ago so I added it and trained a MNIST network with it. I don't have it written down, but I could link a set of simple MNIST classifiers that can be used.

As for the tooltip, I'll try to update it tomorrow.

@gheinrich
Copy link
Contributor

@lukeyeager it is a challenge to implement this for Torch since Torch doesn't have named layers/parameters.

@IgorX2
Copy link
Contributor Author

IgorX2 commented Jan 9, 2016

@lukeyeager I have updated the tooltip. Here is a link to three neural network models that can be used to test the added functionality: http://www.igorsevo.com/File.ashx?type=res&resType=0&title=source+code%5cMNIST-Double.zip.

@@ -256,7 +256,7 @@ def validate_custom_network_snapshot(form, field):
if form.method.data == 'custom':
snapshot = field.data.strip()
if snapshot:
if not os.path.exists(snapshot):
if not all(map(lambda x: os.path.exists(x), snapshot.split(';'))):
Copy link
Member

Choose a reason for hiding this comment

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

Let's strip the whitespace around the separator.

Copy link
Member

Choose a reason for hiding this comment

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

And use : instead of ;.

@IgorX2
Copy link
Contributor Author

IgorX2 commented Jan 17, 2016

@lukeyeager I added the whitespace stripping and changed the delimiter and the tooltip.

@lukeyeager
Copy link
Member

This looks OK to me. Thanks for the updates!

I don't have a signed CLA from you yet, do I? Please follow the instructions here:

https://github.com/NVIDIA/DIGITS/blob/master/CONTRIBUTING.md#pull-requests

lukeyeager added a commit that referenced this pull request Jan 18, 2016
Add option to use multiple pretrained networks
@lukeyeager lukeyeager merged commit ebd0da7 into NVIDIA:master Jan 18, 2016
@lukeyeager
Copy link
Member

Oops, I forgot to ask you to squash your commits into one! Next time.

@lukeyeager
Copy link
Member

And use : instead of ;.
@lukeyeager #498 (comment)

Rats. That was a bad choice of separator on Windows machines. My bad.

Oh, that's unfortunate, DIGITS is confusing the colon in D:\TEST_DIGITS with the separator we use in the presence of multiple pre-trained models.
@gheinrich #49 (comment)

We should probably use os.path.pathsep instead.

# Linux
>>> os.path.pathsep
':'
# Windows
>>> os.path.pathsep
';'

@IgorX2
Copy link
Contributor Author

IgorX2 commented Jun 17, 2016

@lukeyeager: Fixed this and created a pull request #854.

@gheinrich
Copy link
Contributor

Oh, @lukeyeager has already pushed a fix with #851...

@IgorX2
Copy link
Contributor Author

IgorX2 commented Jun 17, 2016

I synced with the master branch, but the changes were not there. Also, there were issues when trying to use this because someone had overwritten the modifications to the validate_custom_network_snapshot method. I corrected that here.

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