Skip to content
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

Static pipelines now handle types with PipelineColumn properties. #1115

Merged
merged 2 commits into from
Oct 3, 2018

Conversation

TomFinley
Copy link
Contributor

  • Update the internal infrastructure to accomodate these types,
  • Update the Roslyn analyzer to accomodate these types.
  • Update the tests so that they exercise this capability.
  • Opportunistically fix some problems with the Roslyn analyzer
    brought up in this work.

Fixes #1085.

@TomFinley
Copy link
Contributor Author

Oh macOS Build_Debug... you kidder.

* Update the internal infrastructure to accomodate these types,
* Update the Roslyn analyzer to accomodate these types.
* Update the tests so that they exercise this capability.
* Opportunistically fix some problems with the Roslyn analyzer
  brought up in this work.
@@ -37,28 +37,28 @@ public static class StaticPipeUtils
/// <typeparam name="TIn">The type parameter for the input type to the data reader estimator.</typeparam>
/// <typeparam name="TDelegateInput">The input type of the input delegate. This might be some object out of
/// which one can fetch or else retrieve </typeparam>
/// <typeparam name="TTupleOutShape"></typeparam>
/// <typeparam name="TOutShape"></typeparam>
Copy link
Contributor

@Zruty0 Zruty0 Oct 2, 2018

Choose a reason for hiding this comment

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

typeparam [](start = 13, length = 9)

remove empty #Resolved

@Zruty0
Copy link
Contributor

Zruty0 commented Oct 2, 2018

namespace Microsoft.ML.StaticPipe

maybe rename to LightGbmStatic while you're at it?.. #Resolved


Refers to: src/Microsoft.ML.LightGBM/LightGBMStatics.cs:12 in 22399ab. [](commit_id = 22399ab, deletion_comment = False)

@Zruty0
Copy link
Contributor

Zruty0 commented Oct 2, 2018

namespace Microsoft.ML.StaticPipe

Statics again. Where were I looking?.. #Resolved


Refers to: src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs:12 in 22399ab. [](commit_id = 22399ab, deletion_comment = False)


Helper(schema, "how.Stuff.hi", BoolType.Instance);
Helper(schema, "how.Stuff.my.Foo", TextType.Instance);
Helper(schema, "how.Stuff.my.Bar", new VectorType(NumberType.Float, 4));
Copy link
Contributor

@Zruty0 Zruty0 Oct 2, 2018

Choose a reason for hiding this comment

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

how.Stuff.my.Bar [](start = 28, length = 16)

'how stuff my bar', hmm...

In the spirit of open-source, should we have a bit more, um, mature naming scheme?.. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change Stuff to Donut?


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

Copy link
Contributor

@Zruty0 Zruty0 left a comment

Choose a reason for hiding this comment

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

:shipit:

@TomFinley
Copy link
Contributor Author

namespace Microsoft.ML.StaticPipe

Sounds good sure.


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


Refers to: src/Microsoft.ML.LightGBM/LightGBMStatics.cs:12 in 22399ab. [](commit_id = 22399ab, deletion_comment = False)

@TomFinley
Copy link
Contributor Author

namespace Microsoft.ML.StaticPipe

Renamed


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


Refers to: src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs:12 in 22399ab. [](commit_id = 22399ab, deletion_comment = False)

@TomFinley
Copy link
Contributor Author

Same test failed... MulticlassTreeFeaturizedLRTest. But with a different bad range. Not sure what's up with that, I definitely didn't touch anything related to that test.


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

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:

@TomFinley TomFinley merged commit ed54ade into dotnet:master Oct 3, 2018
@TomFinley TomFinley deleted the tfinley/Type branch October 4, 2018 16:15
@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 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.

Expand PiGSTy's schema shape type support
3 participants