Model store #1077

IsaacYangSLA · 2016-09-16T19:20:41Z

Depends on #1048, will rebase/squash later after #1048 is merged.

lukeyeager

Just looked through the code - haven't really tried it yet

lukeyeager · 2016-09-19T23:19:47Z

digits/config/store_option.py

+
+    @classmethod
+    def visibility(cls):
+        return config_option.Visibility.HIDDEN


Can you make this DEFAULT instead? Otherwise I have to use python -m digits.config.edit -v to edit it.

Sure. That's also what I got confused about. Thanks for the suggestion.

lukeyeager · 2016-09-19T23:22:01Z

digits/config/store_option.py

+            'url_list': self.url_list
+        }
+
+


I think you're going to want to implement validate() somehow. Right now, it accepts "1234", "foo", etc.

I think I will implement it later in this PR. The code should ensure that URL is supposed to be well-formatted, not random strings.

lukeyeager · 2016-09-19T23:24:05Z

digits/config/store_option.py

+        return True
+
+    def suggestions(self):
+        self.url_list = ['http://localhost/modelstore/']


How is the user going to input a list? This needs to be a string, possibly separated by a separator (a comma seems like a good choice).

lukeyeager · 2016-09-19T23:24:59Z

digits/static/store.html

+    <title>Model Store</title>
+    <!-- Latest compiled and minified CSS -->
+    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css"
+          integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u" crossorigin="anonymous">


Please don't add links that require internet access to download. Either check them into our source tree under digits/static/js/ or pick another library.

lukeyeager · 2016-09-19T23:25:15Z

digits/static/store.html

+    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css"
+          integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u" crossorigin="anonymous">
+    <!-- jQuery (necessary for Bootstrap's JavaScript plugins) -->
+    <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>


This one is already checked into our tree.

lukeyeager · 2016-09-19T23:26:47Z

digits/static/store.html

+            integrity="sha384-Tc5IQib027qvyjSMfHjOMaLkfuWVxZxUPnCJA7l2mCWNIpG9mGCD8wGNIcPD7Txa"
+            crossorigin="anonymous"></script>
+    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.5.8/angular.min.js"></script>
+    <script src="/static/js/store.js"></script>


Please use this format for files stored under static/js/:
https://github.com/NVIDIA/DIGITS/blob/v4.0.0/digits/templates/layout.html#L12

I will look into those three 'scripts' and 'link' tags and modify them accordingly.

lukeyeager · 2016-09-19T23:28:50Z

digits/store/views.py

+# Copyright (c) 2014-2016, NVIDIA CORPORATION.  All rights reserved.
+from __future__ import absolute_import
+
+import flask


Sorry to nitpick, but can you put this import flask below the standard imports? And above the digits imports?
https://www.python.org/dev/peps/pep-0008/#imports

Sure. Sorry for the improper ordering of imports.

lukeyeager · 2016-09-19T23:31:34Z

digits/templates/layout.html

@@ -69,6 +69,7 @@
                    <li><span class="navbar-text"><a target="_blank" href="https://github.com/NVIDIA/DIGITS/tree/master/docs">DIGITS docs</a></span></li>
                    <li><span class="navbar-text"><a target="_blank" href="https://github.com/NVIDIA/DIGITS">DIGITS on github</a></span></li>
                    <li><span class="navbar-text"><a target="_blank" href="https://groups.google.com/forum/#!forum/digits-users">DIGITS User Group</a></span></li>
+                    <li hidden><span class="navbar-text"><a target="_blank" href="https://developer.nvidia.com/digits/nvzoo">NVidia model store for DIGITS</a></span></li>


Can we use NVIDIA here instead of NVidia?

No problem.

lukeyeager

I tried to use this PR, but when I click on the "Update model list" button, nothing happens. No error or anything.

lukeyeager · 2016-09-20T16:40:47Z

digits/views.py

+                'access-model-store': {
+                    'title': 'Retrieve from Model Store',
+                    'id': 'retrieveModelStore',
+                    'url': '/static/store.html',


Let's make this an actual Flask route and not just a link to a static HTML file.

Thanks. Yes, it will be much better to do flask.url_for on this. I changed codes to reflect this.

TimZaman · 2016-09-20T18:38:24Z

I have some trouble setting the custom URL. I do this:
python -m digits.config.edit -v
then I input as Model Store URL List: ['http://foo.bar']

Then in digits/store/views.py I do for debugging print(app.config['store_url_list']) it prints out: ['http://localhost/modelstore/'] ?

So

For me (I might be using it wrong) it does not want to retain the static url
The bogus url should communicate to the front-end that it cannot be reached (404).

IsaacYangSLA · 2016-09-20T21:10:21Z

@TimZaman , my bad. The prompt portion of store codes was broken. I rewrote that portion and it should work now.

TimZaman · 2016-09-20T21:16:07Z

I can see why we want this in the next release, it's really cool!

lukeyeager · 2016-09-20T21:16:01Z

digits/config/store_option.py

-        self.url_list = ['http://localhost/modelstore/']
-        return [prompt.Suggestion(self.url_list, 'H', desc='URL list')]
+        self.url_list = ''
+        return [prompt.Suggestion(self.url_list, 'M', desc='Model Store URL list (comma separated)')]


Please put the format requirements in prompt_message() above. I don't think you actually need any suggestions for now.

I also wondered what should be the suggested URL. Now it's much easier for me for this option.

lukeyeager · 2016-09-20T21:16:17Z

digits/config/store_option.py

+            url_list = value.split(',')
+            for url in url_list:
+                if url is not None and urlparse(url).scheme != "" and not os.path.exists(url):
+                    valid_url_list.append(url)


If it's not valid, throw an error.

Yes. I should throw BadValue and let user to enter the URL again.

The validate method is now throwing BadValue exception whenever it encounters invalid url inside url_list loop.

TimZaman · 2016-09-25T14:31:08Z

I do think we need to make the download progress visible (and a loading indicator when retrieving details from the particular model store)

…Store Enable direct import from Model Store to Pretrained Model Rebase onto master. Use new env-vars for option.

gheinrich

It feels a little unintuitive to have to have to click on Images to load a model from the model store in the pretrained model tab.

gheinrich · 2016-10-10T09:44:44Z

docs/ModelStore.md

+
+
+## Introduction
+Model Store is a new feature in DIGITS.


it might not be new for a very long time :-)

Yes, it's been a long time. Let me remove that. :-)

gheinrich · 2016-10-10T09:54:11Z

docs/ModelStore.md

+
+
+## Create your own Model Store server
+At the top directory, create a master.json file (if your server does not support directory listing).


maybe add a hint to show how to set up a web server. For example:

$ cd .../my-model-store/ $ python2 -m SimpleHTTPServer 8000

gheinrich · 2016-10-10T10:00:25Z

docs/ModelStore.md

+## Browse and import model
+First launch DIGITS.
+Click Pretrained Models tab, and select 'Retrieve from Model Store' under 'Load Model.'
+The new page shows models available in model stores.


can you add screen shots? It's the first thing readers look at before figuring out if they're interested in reading further

I also think screenshots would be much better than sentences after sentences (for most readers like me). I will add some.

gheinrich · 2016-10-10T10:00:47Z

docs/ModelStore.md

+Click Pretrained Models tab, and select 'Retrieve from Model Store' under 'Load Model.'
+The new page shows models available in model stores.
+
+Hoover over the model name shows the complete text in Note field.


small typo hoover->hover

gheinrich · 2016-10-10T11:53:59Z

docs/ModelStore.md

+At the top directory, create a master.json file (if your server does not support directory listing).
+The following is a sample master.json file.
+```
+{'msg':'This is my own model store server.', 'children':['Model01','Model02']}


These should be double quotes - otherwise it's not valid JSON and the parser silently fails (=> you don't see any models in your store)

gheinrich · 2016-10-10T11:55:22Z

digits/templates/store.html

+                            {[ model.info.username ]}
+                        </td>
+                        <td>
+                            <img ng-src="{[value.base_url+model.dir_name]}/logo.png" height="30" width="40"/>


this is showing a broken image if the logo wasn't specified - better not show anything in that case

gheinrich · 2016-10-10T11:56:38Z

digits/templates/store.html

+                    <tr ng-repeat="model in value.model_list | filter: query">
+                        <td>
+                            <div data-toggle="tooltip" title="{[model.info.note]}">
+                                <a href="/store/push?id={[model.id]}"> {[ model.info.name ]} </a>


it's not entirely intuitive that clicking here will push the model - can we show an upload button?

gheinrich · 2016-10-10T12:20:39Z

docs/ModelStore.md

+{'msg':'This is my own model store server.', 'children':['Model01','Model02']}
+```
+Model01 and Model02 are subdirectories containing the actual models.
+Each model must consist one weight file, one info.json file.


also mention that the file pointed to by model_file in info.json (e.g. original.protoxt) must be there.
otherwise a lot of things silently fail (network customization, ...)

Improve UI

IsaacYangSLA · 2016-10-10T21:00:00Z

@gheinrich , really thank you for reviewing the codes and excellent comments. I just pushed the new commits to address each of your comment. Since there are lots of areas to improve, and to save some space, I didn't add my reply to each of your comments.

gheinrich · 2016-10-11T14:12:03Z

Thanks @IsaacYangSLA this looks fine though it still feels like the model store is somewhat hidden and like @TimZaman I feel it would be nice to show progress notifications, when downloading large models.

IsaacYangSLA · 2016-10-11T16:34:58Z

@gheinrich , I believed you also found importing a model took a long time. A progress notification will be really useful to users. I will create another PR for that after this is merged.

lukeyeager

Check your copyright years

lukeyeager · 2016-10-11T16:41:26Z

digits/config/store_option.py

@@ -0,0 +1,37 @@
+# Copyright (c) 2015-2016, NVIDIA CORPORATION.  All rights reserved.


Should be just "2016"

lukeyeager · 2016-10-11T16:41:37Z

digits/store/views.py

@@ -0,0 +1,142 @@
+# Copyright (c) 2014-2016, NVIDIA CORPORATION.  All rights reserved.


Should be just "2016"

lukeyeager · 2016-10-11T16:42:55Z

digits/templates/store.html

@@ -0,0 +1,127 @@
+{# Copyright (c) 2014-2016, NVIDIA CORPORATION.  All rights reserved. #}


Should be just "2016"

lukeyeager · 2016-10-11T16:43:12Z

digits/utils/store.py

@@ -0,0 +1,54 @@
+# Copyright (c) 2014-2016, NVIDIA CORPORATION.  All rights reserved.


Should be just "2016"

IsaacYangSLA · 2016-10-11T18:40:39Z

@lukeyeager , thanks a lot. To be honest, I didn't know the meaning of those years until you pointed them out. It's great to know more.

IsaacYangSLA · 2016-10-11T18:54:30Z

I will merge this and later create another PR for adding the Python layer file support on importing part, which depends on this PR.

Model store

lukeyeager added the enhancement label Sep 19, 2016

lukeyeager self-assigned this Sep 19, 2016

lukeyeager suggested changes Sep 19, 2016

View reviewed changes

lukeyeager removed their assignment Sep 19, 2016

lukeyeager approved these changes Sep 19, 2016

View reviewed changes

lukeyeager suggested changes Sep 20, 2016

View reviewed changes

lukeyeager mentioned this pull request Sep 20, 2016

Use envvars for configuration instead of a file #1091

Merged

lukeyeager suggested changes Sep 20, 2016

View reviewed changes

IsaacYangSLA added 3 commits September 26, 2016 15:54

Add server to server communication to retrieve model list from Model …

1970cfc

…Store Enable direct import from Model Store to Pretrained Model Rebase onto master. Use new env-vars for option.

Handle when labels file not in info.json.

b5a8caa

Document for Model Store

58464a0

lukeyeager mentioned this pull request Oct 3, 2016

ResNet-50 Testing NVIDIA/caffe#194

Closed

gheinrich requested changes Oct 10, 2016

View reviewed changes

Revise document (add screenshots and correct typos)

b41249b

Improve UI

gheinrich approved these changes Oct 11, 2016

View reviewed changes

lukeyeager suggested changes Oct 11, 2016

View reviewed changes

Correct copyright date.

600fdd5

lukeyeager approved these changes Oct 11, 2016

View reviewed changes

IsaacYangSLA merged commit fae5bb8 into NVIDIA:master Oct 11, 2016

lukeyeager mentioned this pull request Oct 19, 2016

"caffe.DataParameter" has no field named "image_data_param" NVIDIA/caffe#256

Closed

lukeyeager mentioned this pull request Oct 31, 2016

Moving jobs to another machine #1035

Closed

SlipknotTN pushed a commit to cynnyx/DIGITS that referenced this pull request Mar 30, 2017

Merge pull request NVIDIA#1077 from IsaacYangSLA/feature/store_link

509ee0a

Model store

lugeluge mentioned this pull request Nov 9, 2017

Can windows use digits store #1877

Open



		## Create your own Model Store server
		At the top directory, create a master.json file (if your server does not support directory listing).

		@@ -0,0 +1,37 @@
		# Copyright (c) 2015-2016, NVIDIA CORPORATION. All rights reserved.

		@@ -0,0 +1,142 @@
		# Copyright (c) 2014-2016, NVIDIA CORPORATION. All rights reserved.

		@@ -0,0 +1,127 @@
		{# Copyright (c) 2014-2016, NVIDIA CORPORATION. All rights reserved. #}

		@@ -0,0 +1,54 @@
		# Copyright (c) 2014-2016, NVIDIA CORPORATION. All rights reserved.

Model store #1077

Model store #1077

Conversation

IsaacYangSLA commented Sep 16, 2016 • edited by lukeyeager Loading

lukeyeager left a comment

Choose a reason for hiding this comment

lukeyeager Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukeyeager left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimZaman commented Sep 20, 2016 • edited Loading

IsaacYangSLA commented Sep 20, 2016

TimZaman commented Sep 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IsaacYangSLA Sep 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimZaman commented Sep 25, 2016

gheinrich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gheinrich Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gheinrich Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

gheinrich Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

IsaacYangSLA commented Oct 10, 2016

gheinrich commented Oct 11, 2016

IsaacYangSLA commented Oct 11, 2016

lukeyeager left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IsaacYangSLA commented Oct 11, 2016

IsaacYangSLA commented Oct 11, 2016

IsaacYangSLA commented Sep 16, 2016 •

edited by lukeyeager

Loading

lukeyeager Sep 19, 2016 •

edited

Loading

TimZaman commented Sep 20, 2016 •

edited

Loading

IsaacYangSLA Sep 20, 2016 •

edited

Loading

gheinrich Oct 10, 2016 •

edited

Loading

gheinrich Oct 10, 2016 •

edited

Loading

gheinrich Oct 10, 2016 •

edited

Loading