Skip to content

Moved TensorFlow samples to its own directory in Samples project. #2429

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

Merged
merged 5 commits into from
Feb 7, 2019

Conversation

zeahmed
Copy link
Contributor

@zeahmed zeahmed commented Feb 6, 2019

This PR addresses following issues

  1. Moved TF sample to TensorFlow folder in samples project.
  2. Renamed the files to meaningful name.
  3. Added a text classification sample.
  4. Updated TensorFlow.Redist nuget to latest in samples project.

@sfilipi, @shmoradims and @rogancarr, let me know if this move can be a problem for any documentation referencing it.

@codecov
Copy link

codecov bot commented Feb 6, 2019

Codecov Report

Merging #2429 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2429      +/-   ##
==========================================
- Coverage   71.26%   71.22%   -0.04%     
==========================================
  Files         785      785              
  Lines      140946   141034      +88     
  Branches    16108    16112       +4     
==========================================
+ Hits       100440   100450      +10     
- Misses      36039    36116      +77     
- Partials     4467     4468       +1
Flag Coverage Δ
#Debug 71.22% <0%> (-0.04%) ⬇️
#production 67.56% <0%> (-0.05%) ⬇️
#test 85.32% <ø> (ø) ⬆️

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@sfilipi
Copy link
Member

sfilipi commented Feb 6, 2019

Sorry I didn't notice it in the first pass, but I think you are trying to add yet another example of how to use the ScoreTensorFlowModel xtension, for text this time?

If that is the case, that is unecessary, because the API reference samples just need one example to illustrate how to use the API. Samples for the Various scenarios, and things you can do with the components go under the machienlearning-samples repo.

Now, if you can turn around the sample, in a way that illustrates one of the other ScoreTensorFlowModel extensions, that would be useful. #Resolved

Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@zeahmed
Copy link
Contributor Author

zeahmed commented Feb 6, 2019

I feel one sample is not enough to show the various different models that can be used with TensorFlowTransform in ML.NET. I am updating the sample to use the different extension of ScoreTensorFlowModel with inspecting the schema of inputs.


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

@@ -43,7 +43,7 @@ internal TensorFlowModelInfo(IHostEnvironment env, TFSession session, string mod
/// <summary>
/// Get <see cref="Schema"/> for complete model. Every node in the TensorFlow model will be included in the <see cref="Schema"/> object.
/// </summary>
internal Schema GetModelSchema()
public Schema GetModelSchema()
Copy link
Contributor Author

@zeahmed zeahmed Feb 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public [](start = 8, length = 6)

This need to be public. Its helpful to get the schema without loading model again. #Resolved

Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@zeahmed zeahmed merged commit e08a329 into dotnet:master Feb 7, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants