Skip to content

Fix runtime exception in MapKeyToVector sample. - #3287

Merged
codemzs merged 1 commit into
dotnet:masterfrom
codemzs:samplefix7
Apr 11, 2019
Merged

Fix runtime exception in MapKeyToVector sample.#3287
codemzs merged 1 commit into
dotnet:masterfrom
codemzs:samplefix7

Conversation

@codemzs

Copy link
Copy Markdown
Member

fixes#3286

{
public uint[] TimeframeVector { get; set; }
public uint[] CategoryVector { get; set; }
public float[] TimeframeVector { get; set; }

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

the other option is leave it as uint[] but add another transform in the pipeline that converts these column types from float to uint but this is not currently possibly because we don't have float to uint converter in Conversions.cs but of course that can be added and we can use C#'s convention for conversion to uint by casting.

CC: @TomFinley

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think changing the type is appropriate -- this is a key-to-vector sample, the output type of key-to-vector is a float vector, so, this change seems appropriate at least to me.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Thanks @TomFinley

@codemzs
codemzs requested a review from TomFinleyApril 11, 2019 02:06
@codecov

codecovBot commented Apr 11, 2019

Copy link
Copy Markdown

Codecov Report

Merging #3287 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #3287 +/- ##
==========================================
- Coverage 72.63% 72.63% -0.01% 
==========================================
Files 807 807 Lines 145129 145129 Branches 16220 16220 ==========================================
- Hits 105417 105414 -3 - Misses 35294 35297 +3 
Partials 4418 4418
FlagCoverage Δ
#Debug72.63% <ø> (-0.01%)⬇️
#production68.17% <ø> (ø)⬆️
#test88.94% <ø> (-0.01%)⬇️
Impacted FilesCoverage Δ
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.7% <0%> (-0.34%)⬇️

@TomFinleyTomFinley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @codemzs !

@codemzs
codemzs requested a review from eerhardtApril 11, 2019 15:00
@codemzs
codemzs merged commit 1e27ead into dotnet:masterApr 11, 2019
codemzs added a commit to codemzs/machinelearning that referenced this pull request Apr 11, 2019
@ghostghost locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for freeto 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.

MapKeyToValue sample throws exception

3 participants

@codemzs@eerhardt@TomFinley