-
Notifications
You must be signed in to change notification settings - Fork 2k
[tfjs-node] Add SavedModel signatureDef loading and deleting API #2217
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
Conversation
nsthorat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 11 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @kangyizhang, @nsthorat, and @pyu10055)
tfjs-node/binding/tfjs_backend.cc, line 987 at r1 (raw file):
tags_ptrs.reset(new const char *[tags_len]); for (size_t i = 0; i < tags_len; ++i) { tags_ptrs[i] = tags_name_array.at(i).c_str();
optional since this probably isnt slow: why not just have split() do this logic too? seems like you're doing 2 passes through the tags array now just to split and then separately to make c_strs()
tfjs-node/binding/tfjs_backend.cc, line 1007 at r1 (raw file):
if (TF_GetCode(tf_status.status) != TF_OK) { NAPI_THROW_ERROR(env, "Faile to load SavedModel: %s",
Failed
tfjs-node/binding/tfjs_backend.cc, line 1036 at r1 (raw file):
TF_DeleteSession(savedmodel_entry->second, tf_status.status); if (TF_GetCode(tf_status.status) != TF_OK) { NAPI_THROW_ERROR(env, "Fail to delete SavedModel: %s",
Failed
tfjs-node/binding/tfjs_binding.cc, line 176 at r1 (raw file):
if (argc < 1) { NAPI_THROW_ERROR(env, "Invalid number of args passed to deleteSavedModel()");
Maybe add something meaningful about how many it expects and how many it got (and elsewhere)
tfjs-node/binding/utils.h, line 309 at r1 (raw file):
// Split a string into a string array with `,` as delimiter. inline std::vector<std::string> split(const std::string &str) {
name this splitByComma? or pass a value to split by?
tfjs-node/binding/utils.h, line 317 at r1 (raw file):
std::string token = str.substr(prev, pos - prev); if (!token.empty()) tokens.push_back(token); prev = pos + 1; // delim.length();
remove this comment?
tfjs-node/src/saved_model.ts, line 36 at r1 (raw file):
// This map is used to keep track of loaded SavedModel metagraph mapping // information. The map key is TFSavedModelSignature id in JavaScript, value is // a turple of path to the SavedModel, metagraph tags, and loaded Session ID in
tuple
tfjs-node/src/saved_model.ts, line 40 at r1 (raw file):
// entries in this map to find if the corresponding SavedModel session has // already been loaded in C++ addon and will reuse it if existing. const loadedSavedModelPathMap = new Map<number, [string, string, number]>();
can these three values be an object with names for readability
tfjs-node/src/saved_model.ts, line 42 at r1 (raw file):
const loadedSavedModelPathMap = new Map<number, [string, string, number]>(); let tfSavedModelSignatureId = 0;
what is this for? should this be prefixed with next?
tfjs-node/src/saved_model.ts, line 183 at r1 (raw file):
/** * Get input and output node names from SavedModel metagraphs info. The * input.output node names will be used when executing a SavedModel signature.
params?
tfjs-node/src/saved_model.ts, line 185 at r1 (raw file):
* input.output node names will be used when executing a SavedModel signature. */ export function getInputAndOutputNodeNameFromMetaGraphInfo(
can you test this in isolation?
tfjs-node/src/saved_model.ts, line 190 at r1 (raw file):
const metaGraphInfo = savedModelInfo[i]; if (tags.length === metaGraphInfo.tags.length && JSON.stringify(tags) === JSON.stringify(metaGraphInfo.tags)) {
if the tags are out of order, what happens? this will fail
tfjs-node/src/saved_model.ts, line 191 at r1 (raw file):
if (tags.length === metaGraphInfo.tags.length && JSON.stringify(tags) === JSON.stringify(metaGraphInfo.tags)) { if (metaGraphInfo.signatureDefs[signature] === undefined) {
== null
tfjs-node/src/saved_model.ts, line 215 at r1 (raw file):
/** * A `tf.TFSavedModelSignature` is a signature loaded from a SavedModel
do we need to have "Signature" here? Can this just be "SavedModel" to parallel LayersModel and GraphModel?
tfjs-node/src/saved_model.ts, line 224 at r1 (raw file):
private readonly jsid: number; private readonly backend: NodeJSKernelBackend; private deleted: boolean;
private deleted = false and you can remove "this.deleted = false;" below
tfjs-node/src/saved_model.ts, line 227 at r1 (raw file):
constructor( sessionId: number, jsid: number, inputNodeNames: string[],
this can be:
constructor(private sessionId: number, private jsid: number...)
and then you can remove all the constructor param instantiation below.
tfjs-node/src/saved_model.ts, line 262 at r1 (raw file):
this.backend.deleteSavedModel(this.sessionId); } else { throw new Error('This SavedModel has been deleted.');
"has already been deleted"
tfjs-node/src/saved_model.ts, line 267 at r1 (raw file):
/** * Placeholder function.
jsdoc this and execute?
tfjs-node/src/saved_model.ts, line 288 at r1 (raw file):
execute(inputs: Tensor|Tensor[]|NamedTensorMap, outputs: string|string[]): Tensor|Tensor[] { throw new Error('Execute() of TFSavedModelSignature is not supported yet.');
lower case e
tfjs-node/src/saved_model.ts, line 293 at r1 (raw file):
/** * Load a signature of a MetaGraph from a SavedModel as `TFSavedModelSignature`.
since this is the public API let's clean up these docs to align more with what we have for loadLayersModel and loadGraphModel. No need to talk about the types here, they get inferred in the documentation. How about something like:
Load a TensorFlow SavedModel from disk.
**talk about what a savedmodel is, link to python docs, etc*
tfjs-node/src/saved_model.ts, line 303 at r1 (raw file):
path: string, tags: string[], signature: string): Promise<TFSavedModelSignature> { ensureTensorflowBackend();
doesnt this happen in nodeBackend()?
tfjs-node/src/saved_model.ts, line 314 at r1 (raw file):
savedModelInfo, tags, signature); let sessionId;
type?
tfjs-node/src/saved_model.ts, line 322 at r1 (raw file):
} } if (typeof sessionId === 'undefined') {
== null
tfjs-node/src/saved_model_test.ts, line 177 at r1 (raw file):
it('load TFSavedModelSignature', async () => { const spy =
s/spy/loadSavedModelMetaGraphSpy, similary elsewhere
tfjs-node/src/saved_model_test.ts, line 180 at r1 (raw file):
spyOn(nodeBackend(), 'loadSavedModelMetaGraph').and.callThrough(); const model = await loadSavedModel( './test_objects/times_three_float', ['serve'], 'serving_default');
where are these coming from? if they're checked in maybe we should put a file extension on them
tfjs-node/src/saved_model_test.ts, line 220 at r1 (raw file):
expect(spy).toHaveBeenCalledTimes(1); model.delete(); expect(spy1).toHaveBeenCalledTimes(1);
expect spy1 to be called 0 times before calling delete
tfjs-node/src/saved_model_test.ts, line 226 at r1 (raw file):
async done => { try { const model = await loadSavedModel(
move the model and model.delete() out of the try. the second model.delete() should be the only thing in the try. similar to the other places
tfjs-node/src/saved_model_test.ts, line 264 at r1 (raw file):
expect(spyOnCallBindingLoad).toHaveBeenCalledTimes(1); signature1.delete(); expect(spyOnNodeBackendDelete).toHaveBeenCalledTimes(1);
every time you make a call you should assert the count for all of the spies (being 0 so you're testing the exact effect of the call)
tfjs-node/src/tfjs_binding.ts, line 52 at r1 (raw file):
numOutputs: number): TensorMetadata[]; // load a SavedModel from a path:
Load a SavedModel from a path.
tfjs-node/src/tfjs_binding.ts, line 55 at r1 (raw file):
loadSavedModel(exportDir: string, tags: string): number; // delete a SavedModel:
Remove a SavedModel from memory.
kangyizhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the detailed review!
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @nsthorat, and @pyu10055)
tfjs-node/binding/tfjs_backend.cc, line 987 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
optional since this probably isnt slow: why not just have split() do this logic too? seems like you're doing 2 passes through the tags array now just to split and then separately to make c_strs()
Good catch. Moved this part to split and make c_str in the same loop.
tfjs-node/binding/tfjs_backend.cc, line 1007 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
Failed
Done.
tfjs-node/binding/tfjs_backend.cc, line 1036 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
Failed
Done.
tfjs-node/binding/tfjs_binding.cc, line 176 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
Maybe add something meaningful about how many it expects and how many it got (and elsewhere)
Done.
tfjs-node/binding/utils.h, line 309 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
name this splitByComma? or pass a value to split by?
Renamed to splitStringByComma.
tfjs-node/binding/utils.h, line 317 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
remove this comment?
Done.
tfjs-node/src/saved_model.ts, line 36 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
tuple
Done.
tfjs-node/src/saved_model.ts, line 40 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
can these three values be an object with names for readability
Done.
tfjs-node/src/saved_model.ts, line 42 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
what is this for? should this be prefixed with next?
Added comments explaining that, this ID is used to keep track of loaded TFSavedModel, so when deleting a TFSavedModel in javascript, the corresponding TF_Session in c++ bindings is deleted if no other TFSavedModel is using this session. Prefix next is added.
tfjs-node/src/saved_model.ts, line 183 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
params?
Done.
tfjs-node/src/saved_model.ts, line 185 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
can you test this in isolation?
Done. And it actually catch a bug.
tfjs-node/src/saved_model.ts, line 190 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
if the tags are out of order, what happens? this will fail
Added a helper function to compare unsorted string arrays. Out of order should not fail here.
tfjs-node/src/saved_model.ts, line 191 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
== null
Done
tfjs-node/src/saved_model.ts, line 215 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
do we need to have "Signature" here? Can this just be "SavedModel" to parallel LayersModel and GraphModel?
Renamed it to TFSavedModel. This is open to discuss. I was thinking that if user want to use multiple signatures from the same SavedModel, the class name should clearly saying that this is a SavedModelSignature. But after a second consideration, this situation is rare and just naming it TFSavedModel aligns better with LayersModel and GraphModel.
tfjs-node/src/saved_model.ts, line 224 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
private deleted = falseand you can remove "this.deleted = false;" below
Done.
tfjs-node/src/saved_model.ts, line 227 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
this can be:
constructor(private sessionId: number, private jsid: number...)and then you can remove all the constructor param instantiation below.
Done.
tfjs-node/src/saved_model.ts, line 262 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
"has already been deleted"
Done.
tfjs-node/src/saved_model.ts, line 267 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
jsdoc this and execute?
Done.
tfjs-node/src/saved_model.ts, line 288 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
lower case e
Done.
tfjs-node/src/saved_model.ts, line 293 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
since this is the public API let's clean up these docs to align more with what we have for loadLayersModel and loadGraphModel. No need to talk about the types here, they get inferred in the documentation. How about something like:
Load a TensorFlow SavedModel from disk. **talk about what a savedmodel is, link to python docs, etc*
Done.
tfjs-node/src/saved_model.ts, line 303 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
doesnt this happen in nodeBackend()?
No. This is a helper function comparing if tfc.getBackend() === 'tensorflow'
tfjs-node/src/saved_model.ts, line 314 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
type?
Done.
tfjs-node/src/saved_model.ts, line 322 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
== null
Done.
tfjs-node/src/saved_model_test.ts, line 177 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
s/spy/loadSavedModelMetaGraphSpy, similary elsewhere
Done.
tfjs-node/src/saved_model_test.ts, line 180 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
where are these coming from? if they're checked in maybe we should put a file extension on them
These are simple SavedModels I exported through Python TF for testing. They are in folder test_objects, which has been ignored by npm. Let me know if some name suffix is preferred.
tfjs-node/src/saved_model_test.ts, line 220 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
expect spy1 to be called 0 times before calling delete
Done.
tfjs-node/src/saved_model_test.ts, line 226 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
move the model and model.delete() out of the try. the second model.delete() should be the only thing in the try. similar to the other places
Done.
tfjs-node/src/saved_model_test.ts, line 264 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
every time you make a call you should assert the count for all of the spies (being 0 so you're testing the exact effect of the call)
Done.
tfjs-node/src/tfjs_binding.ts, line 52 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
Load a SavedModel from a path.
Done.
tfjs-node/src/tfjs_binding.ts, line 55 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
Remove a SavedModel from memory.
Done.
nsthorat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after this! Nice work!
Reviewed 1 of 11 files at r1, 5 of 6 files at r2, 1 of 1 files at r3.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @kangyizhang, and @pyu10055)
tfjs-node/src/saved_model.ts, line 267 at r1 (raw file):
Previously, kangyizhang (Kangyi Zhang) wrote…
Done.
this isnt done yet
tfjs-node/src/saved_model.ts, line 84 at r3 (raw file):
*/ /** * @doc {heading: 'Models', subheading: 'SavedModel', namespace: 'node'}
do you expect users to use this?
tfjs-node/src/saved_model.ts, line 308 at r3 (raw file):
* Load a TensorFlow SavedModel from disk. A SavedModel is a directory * containing serialized signatures and the states needed to run them. For more * information, see this guide: https://www.tensorflow.org/guide/saved_model.
maybe add a note saying this is different than the TensorFlow.js model format
tfjs-node/src/saved_model.ts, line 312 at r3 (raw file):
* @param path The path to the SavedModel. * @param tags The tags of the MetaGraph to load. * @param signature The SignatureDef to load.
where does this come from, and why capital SignatureDef if this is a string? would be good to document this.
tfjs-node/src/saved_model.ts, line 316 at r3 (raw file):
/** @doc {heading: 'Models', subheading: 'SavedModel', namespace: 'node'} */ export async function loadSavedModel( path: string, tags: string[], signature: string): Promise<TFSavedModel> {
can signature be optional and use the default signature if not passed?
also, will we have a model.signatures like python? https://www.tensorflow.org/guide/saved_model
would be good to document that
tfjs-node/src/tfjs_binding.ts, line 52 at r1 (raw file):
Previously, kangyizhang (Kangyi Zhang) wrote…
Done.
note the period
tfjs-node/src/tfjs_binding.ts, line 55 at r1 (raw file):
Previously, kangyizhang (Kangyi Zhang) wrote…
Done.
period here
dsmilkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple small comments. Nice work!
Reviewed 4 of 11 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @kangyizhang, and @pyu10055)
tfjs-node/src/saved_model.ts, line 338 at r2 (raw file):
} const id = nextTFSavedModelId++; const modelSignature =
s/modelSignature/savedModel/
tfjs-node/src/saved_model_test.ts, line 19 at r2 (raw file):
import {nodeBackend} from './nodejs_kernel_backend'; import {getEnumKeyFromValue, getInputAndOutputNodeNameFromMetaGraphInfo, getMetaGraphsFromSavedModel, loadSavedModel, readSavedModelProto} from './saved_model';
instead of importing these from saved_model, import the public facing APIs from './index' forcing you to test public API. You can still import from saved_model here only those methods that you want unit tested, but are not public API.
dsmilkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @kangyizhang, and @pyu10055)
tfjs-node/src/saved_model.ts, line 197 at r3 (raw file):
* @param signature The signature to get input/output node names from. */ export function getInputAndOutputNodeNameFromMetaGraphInfo(
this is not public API, right?
tfjs-node/src/saved_model.ts, line 243 at r3 (raw file):
private backend: NodeJSKernelBackend) {} /** Placeholder function. */
update jsdoc
tfjs-node/src/saved_model.ts, line 248 at r3 (raw file):
} /** Placeholder function. */
update jsdoc
tfjs-node/src/saved_model.ts, line 258 at r3 (raw file):
*/ /** @doc {heading: 'Models', subheading: 'SavedModel'} */ delete() {
call it dispose to align with the rest of the API (layersModel.dispose(), tensor.dispose(), etc)
tfjs-node/src/saved_model.ts, line 276 at r3 (raw file):
/** * Placeholder function.
update jsdoc
tfjs-node/src/saved_model.ts, line 294 at r3 (raw file):
/** * Placeholder function.
update jsdoc
kangyizhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov, @nsthorat, and @pyu10055)
tfjs-node/src/saved_model.ts, line 267 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
this isnt done yet
Oops. Done.
tfjs-node/src/saved_model.ts, line 338 at r2 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
s/modelSignature/savedModel/
Done.
tfjs-node/src/saved_model.ts, line 84 at r3 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
do you expect users to use this?
Yes. This is a helper function for user to know what tags and signatures a TF SavedModel has. This function works similar to tf saved_model_cli show command (https://www.tensorflow.org/guide/saved_model#show_command)
tfjs-node/src/saved_model.ts, line 197 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
this is not public API, right?
This is not public API, only for internal use. It will not be exported in index.ts.
tfjs-node/src/saved_model.ts, line 243 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
update jsdoc
Done.
tfjs-node/src/saved_model.ts, line 248 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
update jsdoc
Done.
tfjs-node/src/saved_model.ts, line 258 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
call it dispose to align with the rest of the API (layersModel.dispose(), tensor.dispose(), etc)
Done.
tfjs-node/src/saved_model.ts, line 276 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
update jsdoc
Done.
tfjs-node/src/saved_model.ts, line 294 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
update jsdoc
Done.
tfjs-node/src/saved_model.ts, line 308 at r3 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
maybe add a note saying this is different than the TensorFlow.js model format
Done. Also added description of what is inside a SavedModel directory.
tfjs-node/src/saved_model.ts, line 312 at r3 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
where does this come from, and why capital SignatureDef if this is a string? would be good to document this.
Updated the jsdoc. This is the name of the SignatureDef. User can take a look of available signatureDef names through above getMetaGraphsFromSavedModel() function.
tfjs-node/src/saved_model.ts, line 316 at r3 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
can signature be optional and use the default signature if not passed?
also, will we have a model.signatures like python? https://www.tensorflow.org/guide/saved_model
would be good to document that
Good point. Both tags and signature should have default value.
For model.signatures: the loaded object is only one signature of the SavedModel, so the loaded model can do inference with input tensors, which aligns with tf.GraphModel and tf.LayersModel. If user want to list all the available signatures of a SavedModel, they can use tf.node.getMetaGraphsFromSavedModel().
tfjs-node/src/saved_model_test.ts, line 19 at r2 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
instead of importing these from
saved_model, import the public facing APIs from './index' forcing you to test public API. You can still import from saved_model here only those methods that you want unit tested, but are not public API.
Done.
tfjs-node/src/tfjs_binding.ts, line 52 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
note the period
Done.
tfjs-node/src/tfjs_binding.ts, line 55 at r1 (raw file):
Previously, nsthorat (Nikhil Thorat) wrote…
period here
Done.
dsmilkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice work! LGTM but it would be great to have a some calls to the new API from the ts integration test (integration/typescript folder)
Reviewed 10 of 10 files at r4.
Reviewable status:complete! 1 of 1 approvals obtained (waiting on @kangyizhang, @nsthorat, and @pyu10055)
tfjs-node/scripts/build-npm.sh, line 31 at r4 (raw file):
# Manual copy src/proto/api_pb.js until both allowJs and declaration are # supported in tsconfig: https://github.com/microsoft/TypeScript/pull/32372 mkdir -p dist/proto && cp src/proto/api_pb.js dist/proto/api_pb.js
Does that test-ts-integration contain a call to the newly added API? Can you do that? That way you will be certain that the protobuf stuff is working for external users.
tfjs-node/src/saved_model.ts, line 84 at r3 (raw file):
Previously, kangyizhang (Kangyi Zhang) wrote…
Yes. This is a helper function for user to know what tags and signatures a TF SavedModel has. This function works similar to tf saved_model_cli show command (https://www.tensorflow.org/guide/saved_model#show_command)
Can you clarify in the jsdoc what this method returns?
tfjs-node/src/saved_model.ts, line 294 at r3 (raw file):
Previously, kangyizhang (Kangyi Zhang) wrote…
Done.
Remove the word "Single" (a typo?)
kangyizhang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @nsthorat and @pyu10055)
tfjs-node/scripts/build-npm.sh, line 31 at r4 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
Does that test-ts-integration contain a call to the newly added API? Can you do that? That way you will be certain that the protobuf stuff is working for external users.
Done. Very good point!
tfjs-node/src/saved_model.ts, line 84 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
Can you clarify in the jsdoc what this method returns?
Done.
tfjs-node/src/saved_model.ts, line 294 at r3 (raw file):
Previously, dsmilkov (Daniel Smilkov) wrote…
Remove the word "Single" (a typo?)
Done. Thanks for catching it.
Add API to load a SavedModel signature. This PR includes:
TFSavedModelSignatureas wrapper of the loaded SavedModel in javascript.TFSavedModelSignaturefrom SavedModel dir.To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is