Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

add execute method for inference model to allow retrieve activation for any internal nodes #1070

Merged
merged 5 commits into from
May 31, 2018

Conversation

pyu10055
Copy link
Collaborator

@pyu10055 pyu10055 commented May 31, 2018

This change is Reviewable

@pyu10055 pyu10055 requested a review from caisq May 31, 2018 17:02
@caisq
Copy link
Collaborator

caisq commented May 31, 2018

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


src/types.ts, line 167 at r1 (raw file):

NamedTensorMap

Can we remove this option? The outputs will always be returned as a single Tensor or an Array of them. Otherwise it's unclear when the return value will be an array and when it'll be a dict (object).


src/types.ts, line 193 at r1 (raw file):

config: ModelPredictConfig,

Can we remove this? See my comment in the doc. I think batching is at a level higher than execute().


Comments from Reviewable

@pyu10055
Copy link
Collaborator Author

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


src/types.ts, line 167 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…
NamedTensorMap

Can we remove this option? The outputs will always be returned as a single Tensor or an Array of them. Otherwise it's unclear when the return value will be an array and when it'll be a dict (object).

it is needed for frozen model, since the order of output is not explicit.


src/types.ts, line 193 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…
config: ModelPredictConfig,

Can we remove this? See my comment in the doc. I think batching is at a level higher than execute().

Done.


Comments from Reviewable

@caisq
Copy link
Collaborator

caisq commented May 31, 2018

:lgtm_strong:

Thanks!


Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


src/types.ts, line 178 at r2 (raw file):

 Optional. 

This is not optional, right? Also, there is no default, right?


src/types.ts, line 183 at r2 (raw file):

multiple outputs.

Add that the order is the same as outputs.


Comments from Reviewable

@pyu10055 pyu10055 merged commit ced050d into master May 31, 2018
@pyu10055 pyu10055 deleted the inference_model branch May 31, 2018 18:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants