[HEXAGON] Initial clip operator for Hexagon - #11549

Merged
kparzysz-quic merged 36 commits into
apache:mainfrom
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged

[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom
jcoplin-quic:jcoplin-quic/clip

Conversation

@jcoplin-quic

@jcoplin-quicjcoplin-quic commented Jun 2, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.

for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.

cc @mehrdadh

@github-actions
github-actionsBot requested a review from mehrdadhJune 2, 2022 18:42
@jcoplin-quic

jcoplin-quic commented Jun 3, 2022

Copy link
Copy Markdown
ContributorAuthor

@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?

@jcoplin-quic

jcoplin-quic commented Jun 14, 2022

Copy link
Copy Markdown
ContributorAuthor

@TejashShah could you please assign some reviewers to this PR

@TejashShah

TejashShah commented Jun 14, 2022

Copy link
Copy Markdown

cc @Lunderberg@cconvey@csullivan

@mehrdadh

mehrdadh commented Jun 16, 2022

Copy link
Copy Markdown
Member

@jcoplin-quic please add some detail to the PR description.

@jcoplin-quic

jcoplin-quic commented Jun 21, 2022

Copy link
Copy Markdown
ContributorAuthor

@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?

mehrdadh
mehrdadh reviewed Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh reviewed Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh requested changes Jun 21, 2022
View reviewed changes

@mehrdadhmehrdadh left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.

@driazati

driazati commented Jun 22, 2022

Copy link
Copy Markdown
Member

hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?

Lunderberg
Lunderberg approved these changes Jun 23, 2022
View reviewed changes

@LunderbergLunderberg 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.

Other than one nitpick, looks good to me!

Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py Outdated
# build the function
with tvm.transform.PassContext(opt_level=3):
func = tvm.build(
sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"

@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.

@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?

@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.

@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.

@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule

@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument

@kparzysz-quic
kparzysz-quic merged commit cc6a85b into apache:mainJun 27, 2022
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati mentioned this pull request Jul 14, 2022
Closed
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
[HEXAGON] Initial clip operator for Hexagon - #11549
Merged
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Jun 27, 2022
Merged
[HEXAGON] Initial clip operator for Hexagon#11549
kparzysz-quic merged 36 commits into
apache:mainfrom 
jcoplin-quic:jcoplin-quic/clip
Conversation
@jcoplin-quic
@jcoplin-quicjcoplin-quic
commented
Jun 2, 2022•

edited

Loading

Copy link


Copy Markdown

Contributor
This PR adds a basic Hexagon specific operator and test for clip. Clip is an elementwise operation to restricts an element between two values, min_value and max_value.
for element in data:
element = max(min(element, max_value), min_value

This operator is useful for DeepLabv3.
cc @mehrdadh
@github-actions
github-actionsBot
requested a review
from mehrdadhJune 2, 2022 18:42
@jcoplin-quic
jcoplin-quic
commented
Jun 3, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm seeing a segmentation fault in one of the CPU integration steps. Is this a known issue? If so is there a fix?
@jcoplin-quic
jcoplin-quic
commented
Jun 14, 2022

Copy link


Copy Markdown

ContributorAuthor
@TejashShah could you please assign some reviewers to this PR
@TejashShah
TejashShah
commented
Jun 14, 2022

Copy link


Copy Markdown

cc @Lunderberg@cconvey@csullivan
@mehrdadh
mehrdadh
commented
Jun 16, 2022

Copy link


Copy Markdown

Member
@jcoplin-quic please add some detail to the PR description.
@jcoplin-quic
jcoplin-quic
commented
Jun 21, 2022

Copy link


Copy Markdown

ContributorAuthor
@mehrdadh I'm not sure why the tests are failing. Would you be able to provide any insight?
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
reviewed
Jun 21, 2022
View reviewed changes
Comment threadtests/python/contrib/test_hexagon/test_clip.py
mehrdadh
mehrdadh
requested changes
Jun 21, 2022
View reviewed changes
@mehrdadhmehrdadh
left a comment
•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

The error on ci_cpu is because of the missing fixture, but on ci_hexagon it has error on tests/python/contrib/test_hexagon/test_clip.py::TestClipSlice::test_clip_slice[nhwc-8h2w32c2w-2d-input_shape0-output_shape0-nhwc-8h2w32c2w-2d-float16-0.1-0.5]. That is introduced by the this PR.
@driazati
driazati
commented
Jun 22, 2022

Copy link


Copy Markdown

Member
hey @jcoplin-quic, CI was busted when you pushed so the build never started, could you rebase/merge with main and push to your PR branch again?
Lunderberg
Lunderberg
approved these changes
Jun 23, 2022
View reviewed changes
@LunderbergLunderberg
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.

Other than one nitpick, looks good to me!
Comment threadtests/python/contrib/test_hexagon/topi/test_clip.py
Outdated

 # build the function
 with tvm.transform.PassContext(opt_level=3):
 func = tvm.build(
 sch, [A, M], tvm.target.Target(target_hexagon, host=target_hexagon), name="clip"
@LunderbergLunderbergJun 23, 2022

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.

The [A, M] argument can be removed. See comment here.
@jcoplin-quicjcoplin-quicJun 24, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Hi @Lunderberg removing this argument is causing the build to fail. It's not immediately evident to me why. Would you have any insight?
@csullivancsullivanJun 24, 2022

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.

Hi @jcoplin-quic, with TIR schedules you'll want to pass in the module, sch.mod, in which case you don't need the tensor list as @Lunderberg pointed out.
@LunderbergLunderbergJun 27, 2022

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.

Agreed, the IRModule should be passed to tvm.build, rather than the tvm.tir.Schedule. I've submitted #11913 to improve the error message, as I can definitely see the confusion when the error message doesn't distinguish between expected a te.Schedule or tir.Schedule.
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Thanks @csullivan and @Lunderberg . At line 85, I assign tir_schedule.mod to sch. So sch should be IRModule, not tvm.tir.Schedule
@jcoplin-quicjcoplin-quicJun 27, 2022

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I think it had to do with optional argument target being treated as a positional argument
@kparzysz-quic
kparzysz-quic
merged commit cc6a85b
into
apache:mainJun 27, 2022
blackkker pushed a commit
to blackkker/tvm
that referenced
this pull request
Jul 7, 2022
@jcoplin-quic@blackkker
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
0af75ba
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

mikeseven pushed a commit
to mikeseven/tvm
that referenced
this pull request
Sep 27, 2023
@jcoplin-quicMikael Sevenier
[HEXAGON] Initial clip operator for Hexagon (apache#11549)
260e36d
* [HEXAGON] Initial clip operator for Hexagon
* Changes to utils and infra for pylint
* Remove unused import
* Use tvm.testing.main()
* Address pylint error
* Fix incorrect function call
* Changes to calls to transform_numpy
* Add newline at end of file
* Add requires_hexagon and rename under topi
* Whitespace fix and reduce input size
* Remove te tensor arguments
* Correct call to tvm.build
* Run black formatting
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@LunderbergLunderbergLunderberg approved these changes
@mehrdadhmehrdadhmehrdadh approved these changes
@csullivancsullivanAwaiting requested review from csullivan

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@jcoplin-quic@TejashShah@mehrdadh@driazati@Lunderberg@csullivan@kparzysz-quic