Model Dowload Buttons #719
Conversation
…Download Buttons.
…nks into the Model View and making slight changes for readablity.
* Use builder pattern for Parameter (deepjavalibrary#661) * Make XavierInitializer default value & Improve setInitializer (deepjavalibrary#664) * Refactor initialize (deepjavalibrary#675) * Remove NDManager on getOutputShapes (deepjavalibrary#710)
central/src/main/java/ai/djl/serving/central/handler/ModelDownloadHandler.java
Show resolved
Hide resolved
| @@ -250,6 +252,9 @@ export default function ModelView(props) { | |||
| : <DynForm data={noSynset}/> | |||
| } | |||
| </TabPanel> | |||
| <TabPanel value={index} index={6} className={classes.tabpanel}> | |||
| <ModelDownloadButtons modelName={props.model.name}/> | |||
There was a problem hiding this comment.
Maybe it would be better to add a Menu with actions which are shown all the time instead of adding a tab with download action.
If no more interactions for models are planned this is fine. but I am just wondering in case more actions to be included in future, if we want to have 1 tab per action.
ffrom a usability perspective it is usally a better approach to show all possible actions on a central place.
As I said just a thought in case that more interactions with the model are planned.
There was a problem hiding this comment.
I like this idea actually. I wanted to change what the UI looked like, but thought it would be messy to add a change like that to what should be a simple PR. Also, I didn't want to delay a PR while the actual functionality hasn't been checked.
There was a problem hiding this comment.
I also am not completely sure of the end goal for this project, so it could be very much the plan to add more actionable items and if that's the case I think that you surely had a better solution with the action button.
There was a problem hiding this comment.
@anfee1 speaking of future plans for this sub-project, I was also thinking if it might be better to do the download through the central server instead of just sending the link to the web-client.
At the moment every web-client/browser needs access through firewalls/proxies to download the model from the original URL as well as to central.
Instead of that it would be possible that just the central-server is configured to access all necessary network-resources.
The client could just call an URL of the central-server and the server would stream the model-download to the client
I hope you know what I mean :-) ?
This would open the door to some future improvements like:
- Authentication/Authorisation of downloads managed by central. (OAuth-plugin, Kerberos-plugin)
- central could be used as a Mirror or for Caching of remote models - similar to how Nexus can be configured when using maven. Nexus is used often this way for caching or to control which artifacts are used inside of a company.
- uploading models with central without granting access for all web-clients to whatever storage central is using. so different storage strategies would be possible.
- single point to configure firewall access. Would also be nice for cloud based services using models.
just a few ideas from a DevOp perspective
But as I said depends on future plans for this project and can easily be added later on.
There was a problem hiding this comment.
I am having trouble thinking of another action that would go in a separate button. Maybe we make a more generic "Get Model" tab. Then, it could include the download button and some other actions about getting the model like a sample criteria snippet.
@ebamberg I think what you are saying about proxying the download through the model server makes sense (at least as an option). We can probably put that in a later PR, though. Do you want to open an issue about it?
There was a problem hiding this comment.
@zachgk yes sure I can open a issue about that. That sounds sensible. Can I do that on the project tab?
for other actions: I can think of "deploy model" (see serving, deploy to 1 or more remote serving instances, or when central runs as plugin on serving-server deploy to "itself") , "upload model", "create docker" , "remove model", "invalidate model" to flag a model as "don't use anymore for new projects" or so. just a few ideas, maybe some more fancy actions like "convert model" to convert a model from keras to TFLite for example.
There was a problem hiding this comment.
Github requires write permissions to add it on the project tab. If you go ahead and create a normal issue, I will add it to the project tab for you (feel free to add other issues too).
There was a problem hiding this comment.
Github requires write permissions to add it on the project tab. If you go ahead and create a normal issue, I will add it to the project tab for you (feel free to add other issues too).
already done.
I have created a issue for this: #735
* Fixing TrainingListener documentation * Fixing PR reviews
Change-Id: I9eccc84b0c34652e50c5fe5a4fe42f2b82d65a3d
central/src/main/java/ai/djl/serving/central/classes/ModelLink.java
Outdated
Show resolved
Hide resolved
central/src/main/java/ai/djl/serving/central/classes/ModelLink.java
Outdated
Show resolved
Hide resolved
central/src/main/java/ai/djl/serving/central/classes/ModelLink.java
Outdated
Show resolved
Hide resolved
central/src/main/java/ai/djl/serving/central/classes/ModelLink.java
Outdated
Show resolved
Hide resolved
central/src/main/java/ai/djl/serving/central/classes/ModelLink.java
Outdated
Show resolved
Hide resolved
central/src/main/java/ai/djl/serving/central/responseencoder/HttpRequestResponse.java
Outdated
Show resolved
Hide resolved
| @@ -250,6 +252,9 @@ export default function ModelView(props) { | |||
| : <DynForm data={noSynset}/> | |||
| } | |||
| </TabPanel> | |||
| <TabPanel value={index} index={6} className={classes.tabpanel}> | |||
| <ModelDownloadButtons modelName={props.model.name}/> | |||
There was a problem hiding this comment.
I am having trouble thinking of another action that would go in a separate button. Maybe we make a more generic "Get Model" tab. Then, it could include the download button and some other actions about getting the model like a sample criteria snippet.
@ebamberg I think what you are saying about proxying the download through the model server makes sense (at least as an option). We can probably put that in a later PR, though. Do you want to open an issue about it?
Codecov Report
@@ Coverage Diff @@
## master #719 +/- ##
============================================
+ Coverage 68.83% 69.11% +0.27%
- Complexity 3975 3983 +8
============================================
Files 463 462 -1
Lines 18501 18636 +135
Branches 1992 1998 +6
============================================
+ Hits 12736 12881 +145
+ Misses 4733 4730 -3
+ Partials 1032 1025 -7 Continue to review full report at Codecov.
|
Change-Id: I66d54aa496cccbb9e8c0a89eeaa458605958d9c6
* paddlepaddle CN notebook * install font Change-Id: I2d749e617b0bf78ecbcd168b82c53a1fab49a2c0 * refactor on name Change-Id: I9e379eee51ceae16391850b3ba9782acb04c4021 * Refine the text Co-authored-by: gstu1130 <[email protected]>
* add EI documentation * fix pmd rules Change-Id: Ieee5577c26f6df2843781f8f9180de35069a5de3
* allow pytorch stream model loading * updates Change-Id: Ibc26261b90de673712e90de0d640a8f32f23763e
Change-Id: I6a31d8b0b955f2dbb762220b101e3928a34699c1
The MemoryScope reveals a number of shortcomings within the DJL memory
management. While the MemoryScope is deleted, many of them are fixed as part of
this PR.
First, the NDManager.{attach, detach} were renamed to xxxInternal. This is to
differentiate them from the attach and detach methods that are intended to be used.
There are two new concepts in memory management. An NDResource interface was
created to combine the concepts of managed memory that was used in NDArray and
NDList. It could also be used in more classes in the future. This includes the
getManager, attach, and detach.
Within the NDManager, it gains a second "management convention". The first
convention of normal resources are added to the manager and then closed when the
manager closes. This works for small numbers of things on the NDArray, but not
when operations transitively create. So, the second convention is a
tempResource. Instead of freeing them when the manager is closed, they are
returned to their original manager. This is used to create a temporary scope, do
operations within it, and then the inputs and return value are returned to the
parent while the intermediate work is cleaned. This also matches the concepts of
ownership/borrowing as well.
Using these, a few additional helper methods were created. There is
`NDManager.from(resource)` to ease creation of managers based on a resource.
There is also `scopeManager.ret(returnValue)` to help with returning values
outside of the scopeManager. Lastly, there is a `scopeManager.{temp,}AttachAll`
to attach a number of resources to a manager within a single call.
Using these improvements, the new method were applied to the old locations where
MemoryScope was used as well as an additional case in NDManagerEx.
Also, the old attach methods were altered to be `void`. Because the return
values are no longer used anywhere and are not as necessary in the current
scheme, I figured it would simplify things. It also helps for things like
`NDList.attach` which does not have a single original NDManager when attaching.
Change-Id: I91d109cd14d70fa64fd8fffa0b50d88ab053013e
The application was changed to the more accurate softmax_regression (matching the terminology from the D2L book). Change-Id: I1f69f005bbe38b125f2709c2988d06c14eebb765
* remove methods that already defined in the NDArrayAdapter Change-Id: I01cc03a7f5b427bf31c6b3fd8d2136f2a27fe93b * refactor toString Change-Id: Iea22b16e1daa9f759b55c1a8b8b85536482e551a * remove sparse NDArray Change-Id: Icb44096519775f54cb32cc768c14f49e33dc7ea5 * fix test Change-Id: Icef580ed77e7bba22864ce44577de3cba51e3e41
* S3 Cache Engine This creates the S3 Cache engine. It is put into the same cache plugin by expanding the DDB plugin to handle it as well. Alongside this, there is some work done to synchronize the efforts on cache engines. A new BaseCacheEngine class is created to contain some common logic between the cache engines. The former tests for the DDB cache engine were generalized a bit and turned into a suite that can be and is run for all of the three supported cache engines. This ensures (and fixes) some inconsistencies in behavior among the cache engines. Also important is that it adds a new test dependency on localstack (https://localstack.cloud/). This runs a local AWS clone inside a docker container and is used to verify the running of the S3 cache. * Fix typo and add skip for failure to start localstack
Description
This gives the functionality of providing download URIs that are taken from the model, converts them to URLs, and then feeds it back to the model view in the form of a clickable button.
Interesting edge cases to note here:
A way needs to be found to clear the model links on the front end so when you switch from model to model the unused links will disappear. More will be explained in an issue that I will link.
Special Thanks to @ebamberg for the help/insight. #694
Fixes:#633