Exposed TensorFLow session as TensorFlowModelInfo class #1191

zeahmed · 2018-10-09T01:09:23Z

This PR fixes #1157 where loaded TFSession is wrapped into TensorFlowModelInfo class.
In addition to that, the TensorFlowModelInfo provides following methods to query schema

GetModelSchema(): Get all the information in the model as ISchema object.
GetInputSchema(): Get only input related information from the model as ISchema object. It is useful for the case when the graph is very large and user cannot locate inputs in such a large graphs.

Please see the modified test for more insights.

…chema and TFTransform.

Ivanidzo4ka · 2018-10-09T05:45:23Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+using Microsoft.ML.Runtime;
+using Microsoft.ML.Runtime.Data;
+
+namespace Microsoft.ML.Transforms.TensorFlow


I would probably put it into same namespace as tensorflow transform, i.e. Microsoft.ML.Transforms.

Also requires update in documentation.
Would be nice to explain user in which case he need to use simple constructor and in which this Context object. #Closed

Zruty0 · 2018-10-09T17:00:20Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+{
+    /// <summary>
+    /// This class holds the information related to TensorFLow model and session.
+    /// It provides a convenient way to query model schema


chema [](start = 53, length = 5)

please use correct punctuation #Closed

Zruty0 · 2018-10-09T17:00:52Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+    /// </summary>
+    public class TensorFlowModelContext
+    {
+        internal TFSession Session { get; private set; }


private set; [](start = 42, length = 12)

use just { get; } for read-only properties #Closed

Zruty0 · 2018-10-09T17:01:16Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+        internal TFSession Session { get; private set; }
+        public string ModelPath { get; private set; }
+
+        private readonly IHostEnvironment _host;


_host [](start = 42, length = 5)

either rename to _env or register an IHost #Closed

Zruty0 · 2018-10-09T17:01:45Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+        }
+
+        /// <summary>
+        /// Get <see cref="ISchema"/> for only those nodes which are marked "PlaceHolder" in the TensorFlow model.


PlaceHolder [](start = 77, length = 11)

is it PlaceHolder or Placeholder ? #Closed

Zruty0 · 2018-10-09T17:02:16Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+    ///     - Get complete schema by calling <see cref="GetModelSchema()"/>
+    ///     - Get schema related to model input by calling <see cref="GetInputSchema()"/>
+    /// </summary>
+    public class TensorFlowModelContext


TensorFlowModelContext [](start = 17, length = 22)

can we think of a better name than Context ? We are trying to limit ML.NET to just one context, called MLContext #Closed

Maybe something along the lines of 'handle', or I don't know, Info ?

In reply to: 223784755 [](ancestors = 223784755)

Ivanidzo4ka · 2018-10-09T19:35:11Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

            {
-                var imageHeight = 32;
-                var imageWidth = 32;
+                var tensorFlowContext = TensorFlowUtils.LoadTensorFlowModel(env, model_location);


tensorFlowContext [](start = 20, length = 17)

tensorFlowModel maybe? Since name of the class in not Context anymore #Closed

Ivanidzo4ka · 2018-10-09T19:36:15Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

        private sealed class Reconciler : EstimatorReconciler
        {
            private readonly string _modelFile;
+            private readonly TensorFlowModelInfo _tensorFlowModelContext;


_tensorFlowModelContext [](start = 49, length = 23)

_tensorFlowModelInfo maybe? #Closed

Zruty0

Ivanidzo4ka · 2018-10-09T19:40:32Z

src/Microsoft.ML.TensorFlow/TensorflowTransform.cs

            public Reconciler(string modelFile)
            {
                Contracts.AssertNonEmpty(modelFile);
                _modelFile = modelFile;


_modelFile [](start = 16, length = 10)

I would probably get rid of _modelFile and just use _tensorFlowModelContext since you can create it from modelFile anyway.
Less complicated I think from code standpoint #WontFix

Hmm...I tried and think that its better to keep it as-is. The reason is because TensorFlowUtils.LoadTensorFlowModel requires IHostEnvironment which is not available until Reconcile method is hit.
Let me know if you are thinking of some other solution?

In reply to: 223836239 [](ancestors = 223836239)

abgoswam · 2018-10-09T19:58:19Z

src/Microsoft.ML.TensorFlow/TensorFlow/TensorFlowModelContext.cs

+namespace Microsoft.ML.Transforms.TensorFlow
+{
+    /// <summary>
+    /// This class holds the information related to TensorFLow model and session.


L [](start = 59, length = 1)

nit: typo #Resolved

Ivanidzo4ka

zeahmed added 21 commits September 26, 2018 14:45

Making TensorflowTransform trainable.

22d08d6

Merge remote-tracking branch 'upstream/master' into tf_training

1fc443a

Added model files and script to create model in python.

ef67dd1

Added functionality to serialized TensorFlow model after retraining.

c1eafa6

Added comments. Added checking for parameters and types.

07ae61c

Merge remote-tracking branch 'upstream/master' into tf_training

6ec4a5a

Addressed reviewers' comments.

36bf551

Addressed reviewers' comments.

570bd02

Added more tests...

3cc20e4

Merge remote-tracking branch 'upstream/master' into tf_training

49bdf59

Fixed failing tests.

a9a90ab

Using progress channel to report metrics.

345c504

Moved models to models repo.

883f9b4

Removed extra array copy.

18c70f3

Addressed reviewers' comments.

8ea8e9d

Addressed reviewers' comments.

735f7ca

Addressed reviewers' comments.

9c07a53

Exposed TensorFlowModelContext so that same session can be used for s…

8943545

…chema and TFTransform.

Added Pigsty extensions.

0d02fb1

Changes tests to use model schema to set image height and width.

fe69f16

Merged and resolved conflicts.

eb45aed

zeahmed self-assigned this Oct 9, 2018

zeahmed requested review from Ivanidzo4ka, Zruty0, abgoswam, markusweimer and yaeldekel October 9, 2018 01:09

Ivanidzo4ka reviewed Oct 9, 2018

View reviewed changes

Zruty0 reviewed Oct 9, 2018

View reviewed changes

Addressed reviewers' comments.

700cc9e

Ivanidzo4ka reviewed Oct 9, 2018

View reviewed changes

Zruty0 approved these changes Oct 9, 2018

View reviewed changes

Ivanidzo4ka reviewed Oct 9, 2018

View reviewed changes

abgoswam reviewed Oct 9, 2018

View reviewed changes

Addressed reviewers' comments.

0b73184

Ivanidzo4ka approved these changes Oct 9, 2018

View reviewed changes

zeahmed changed the title ~~Exposed TensorFLow session as TensorFlowModelContext class~~ Exposed TensorFLow session as TensorFlowModelInfo class Oct 9, 2018

Renamed the file according to class name "TensorFlowModelInfo.cs".

e5cc722

zeahmed merged commit 8ca1c93 into dotnet:master Oct 9, 2018

ghost locked as resolved and limited conversation to collaborators Mar 28, 2022

Exposed TensorFLow session as TensorFlowModelInfo class #1191

Exposed TensorFLow session as TensorFlowModelInfo class #1191

Uh oh!

Conversation

zeahmed commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ivanidzo4ka Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zruty0 left a comment

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Oct 9, 2018 • edited by zeahmed Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zeahmed Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

abgoswam Oct 9, 2018 • edited by zeahmed Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zeahmed commented Oct 9, 2018 •

edited

Loading

Ivanidzo4ka Oct 9, 2018 •

edited

Loading

Zruty0 Oct 9, 2018 •

edited

Loading

Zruty0 Oct 9, 2018 •

edited

Loading

Zruty0 Oct 9, 2018 •

edited

Loading

Zruty0 Oct 9, 2018 •

edited

Loading

Zruty0 Oct 9, 2018 •

edited

Loading

Ivanidzo4ka Oct 9, 2018 •

edited

Loading

Ivanidzo4ka Oct 9, 2018 •

edited

Loading

Ivanidzo4ka Oct 9, 2018 •

edited by zeahmed

Loading

abgoswam Oct 9, 2018 •

edited by zeahmed

Loading