-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Introduce order for pixel extraction #2602
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
Ivanidzo4ka
merged 16 commits into
dotnet:master
from
Ivanidzo4ka:Ivanidze/ImagePixelExtractorOrder
Feb 25, 2019
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
329ac5e
Introduce order for pixel exctraction
2a4deb1
shuffle defaults to estimator, slightly better comments
aadf930
make it build
1a17ee0
make it build part 2
0a1b626
Merge branch 'master' into Ivanidze/ImagePixelExtractorOrder
af9b968
Entrypoints test
f91bc44
merge with master
63081f2
fix entrypoints
df4e9fa
update wording
cee009e
order/scale
f9814b6
All that harsh feedback!
0a3d5d3
two spaces
7e0140b
more changes
eec5c6d
Merge branch 'master' into Ivanidze/ImagePixelExtractorOrder
55e6ad0
One curly boy
ff9aa7d
different name for model
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think your code now has the opposite logic.
Would prefer to be consistent with the above extension here too. If you could also reorder them so that the first one applied comes first that would be great. #Resolved
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.
What do you mean?
In reply to: 259550143 [](ancestors = 259550143)
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.
Are you talking about scale offset or defaultAlpha?
In reply to: 259550340 [](ancestors = 259550340,259550143)
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.
Sorry I think codeflow did not pick up the right location for this. I meant to comment on
scale
andoffset
.machinelearning/src/Microsoft.ML.ImageAnalytics/VectorToImageTransform.cs
Lines 415 to 419 in cee009e
It seems that you are first scaling and the applying the offset. #Resolved
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.
So I can do this: P-pixel x -VectorValue
p= xscale1-offset1
x =pscale2-offset2
this way I need to solve equation in order to pick up scale1 and offset1 if I know scale2 and offset2.
or I can have
x=(p-offset1)scale1
p = xscale2 - offset2
and relationship between offset1 = -offset2 and scale1 = 1/scale2
So in one transform i apply scale first, in another i apply offset.
I can make them same order in their parameters, but since I'm specifying which applied first and which second, it looks weird then you put one which applied second to first in argument list.
In reply to: 259551787 [](ancestors = 259551787)
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.
Thank you for explaining the logic it makes sense like this!
In reply to: 259553517 [](ancestors = 259553517,259551787)