[microNPU][2a] Add CascaderGraph for cascading analysis - #9469

Merged
Mousius merged 6 commits into
apache:mainfrom
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged

[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom
mbaret:ethosu-cascader-2

Conversation

@mbaret

@mbaretmbaret commented Nov 8, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

RFC: apache/tvm-rfcs#37
Issue: #9429

A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.

The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021
Closed
Merged
@mbaret
mbaretforce-pushed the ethosu-cascader-2 branch 2 times, most recently from 3d4b396 to 22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret marked this pull request as ready for review December 1, 2021 14:15
@mbaret

mbaret commented Dec 1, 2021

Copy link
Copy Markdown
ContributorAuthor

cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm

jacobbohlin
jacobbohlin reviewed Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin reviewed Dec 20, 2021
View reviewed changes

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

LGTM - Just a question and very minor suggestions.

Comment threadsrc/contrib/ethosu/cascader/graph.h Outdated
Comment threadsrc/contrib/ethosu/cascader/graph.h
public:
void VisitAttrs(AttrVisitor* v);

/*! \return The shape of the tensor */

@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?

@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.

@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?

@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.

@jacobbohlinjacobbohlin mentioned this pull request Dec 20, 2021
Merged
manupak
manupak reviewed Dec 21, 2021
View reviewed changes

@manupakmanupak left a comment •
edited
Loading

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.

Generally looks good.

There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.

Others we could do in a follow up.

Comment threadcmake/modules/contrib/EthosU.cmake Outdated
file(GLOB COMPILER_ETHOSU_SRCS
CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*

@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.
See here : #9630

@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.

Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin approved these changes Jan 5, 2022
View reviewed changes

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

LGTM

@mbaret

mbaret commented Jan 5, 2022

Copy link
Copy Markdown
ContributorAuthor

@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)

Mousius
Mousius approved these changes Jan 5, 2022
View reviewed changes

@MousiusMousius left a comment

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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸

@Mousius
Mousius merged commit 72d3efe into apache:mainJan 5, 2022
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati mentioned this pull request Jul 14, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics
+1 more reviewer
@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak
, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak

, '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
[microNPU][2a] Add CascaderGraph for cascading analysis - #9469
Merged
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Jan 5, 2022
Merged
[microNPU][2a] Add CascaderGraph for cascading analysis#9469
Mousius merged 6 commits into
apache:mainfrom 
mbaret:ethosu-cascader-2
Conversation
@mbaret
@mbaretmbaret
commented
Nov 8, 2021•

edited

Loading

Copy link


Copy Markdown

Contributor
RFC: apache/tvm-rfcs#37

Issue: #9429
A CascaderGraph augments a TE graph with additional information needed by the cascading algorithms. This includes defining a strict ordering on the operators as well as including all the Propagators needed to do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts and Tensors. A Part is an augmented operator which includes the Propagators and a Tensor is similar to a TE tensor but stores additional information like compression ratio.

This was referenced Nov 8, 2021

Closed

 Merged

@mbaret
mbaretforce-pushed
the
ethosu-cascader-2
branch
2 times, most recently
from
3d4b396 to
22447b4CompareDecember 1, 2021 14:14
@mbaret
mbaret
marked this pull request as ready for review
December 1, 2021 14:15
@mbaret
mbaret
commented
Dec 1, 2021

Copy link


Copy Markdown

ContributorAuthor
cc @jacobbohlin@NicolaLancellotti@csullivan@manupa-arm
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
Comment threadsrc/contrib/ethosu/cascader/common.h
jacobbohlin
jacobbohlin
reviewed
Dec 20, 2021
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM - Just a question and very minor suggestions.
Comment threadsrc/contrib/ethosu/cascader/graph.h
Outdated

Comment threadsrc/contrib/ethosu/cascader/graph.h
 public:
 void VisitAttrs(AttrVisitor* v);

 /*! \return The shape of the tensor */
@jacobbohlinjacobbohlinDec 20, 2021

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.

What's the purpose of all these functions, as opposed to having public class members?
@mbaretmbaretDec 20, 2021

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.

They reflect the members into Python.
@jacobbohlinjacobbohlinDec 21, 2021

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 reason I'm asking is because PerformanceInfo reflects its members by making them public and wondered why it's done differently here. Is it required for immutability since Tensor has a MUTABLE_OBJECT_REF?
@mbaretmbaretDec 21, 2021

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.

Making the members public isn't enough on its own to reflect them into Python, PerformanceInfo still defines VisitAttrs. Tensor having MUTABLE_OBJECT_REF is so that AddProducer/Consumer work as these are non-const methods.
@jacobbohlinjacobbohlin
mentioned this pull request
Dec 20, 2021
 Merged

manupak
manupak
reviewed
Dec 21, 2021
View reviewed changes
@manupakmanupak
left a comment
•

edited

Loading

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.

Generally looks good.
There is a bit of a critical issue that we need to fix here which is that the object definitions should always be compiled irrespective of the cmake config.
Others we could do in a follow up.
Comment threadcmake/modules/contrib/EthosU.cmake
Outdated

 file(GLOB COMPILER_ETHOSU_SRCS
 CONFIGURE_DEPENDS src/relay/backend/contrib/ethosu/*
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*)
 CONFIGURE_DEPENDS src/contrib/ethosu/cascader/*
@manupakmanupakDec 21, 2021

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.

If we have source files that defines object definitions, we need to include the irrespective of cmake config.

See here : #9630
@mbaretmbaretDec 22, 2021

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.

Understood, good catch. I've added these files to the 'else' condition here.
Comment threadpython/tvm/contrib/ethosu/cascader/graph.py
Comment threadsrc/contrib/ethosu/cascader/graph.cc
@mbaret
[microNPU][2a] Add CascaderGraph for cascading analysis
e9df3ed
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
Change-Id: I0d147ef649e4187678ff13208d7921e52aa78238
@mbaret
Fix test guards
e56a2ad
Change-Id: Icb8f0fd19645f0c0b2730e8db0d2d2b41a34e2c1
@mbaret
Address review comments
fdd9000
Change-Id: I0a541c30813a97e570a5c49fa501daf2b563e66a
@mbaret
Fix lint
4f37de1
Change-Id: I610a1337c6a7ed05d5f19ffddf13913aa85df46f
@mbaret
Fix size_t to int cast
324497c
Change-Id: I7e0089d55c3bc4eb5d773177de14abf98b5f63a0
@mbaret
Fix tensor size type
17942f3
Change-Id: Idacf97a9672698d18421f17f4f5b9f62563b1509
jacobbohlin
jacobbohlin
approved these changes
Jan 5, 2022
View reviewed changes
@jacobbohlinjacobbohlin
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.

LGTM
@mbaret
mbaret
commented
Jan 5, 2022

Copy link


Copy Markdown

ContributorAuthor
@leandron@Mousius could you please take a look and merge if you think this is OK to go in? Thanks :)
Mousius
Mousius
approved these changes
Jan 5, 2022
View reviewed changes
@MousiusMousius
left a comment


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.

LGTM @mbaret, unsure on liberal use of @property rather than defining properties but I don't see any clear guidance so I think we should move forwards with this great contribution 😸
@Mousius
Mousius
merged commit 72d3efe
into
apache:mainJan 5, 2022
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 7, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
d1301dc
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
ylc pushed a commit
to ylc/tvm
that referenced
this pull request
Jan 13, 2022
@mbaret@ylc
[microNPU][2a] Add CascaderGraph for cascading analysis (apache#9469)
accfdcf
A CascaderGraph augments a TE graph with additional
information needed by the cascading algorithms. This
includes defining a strict ordering on the operators
as well as including all the Propagators needed to
do the affine analysis of cascades.
The CascaderGraph consists of two object types, Parts
and Tensors. A Part is an augmented operator which
includes the Propagators and a Tensor is similar to a
TE tensor but stores additional information like
compression ratio.
@driazatidriazati
mentioned this pull request
Jul 14, 2022

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@manupakmanupakmanupak left review comments
@MousiusMousiusMousius approved these changes
@areuschareuschAwaiting requested review from areusch
@comaniaccomaniacAwaiting requested review from comaniac
@icemelonicemelonAwaiting requested review from icemelon
@jroeschjroeschAwaiting requested review from jroesch
@junrushaojunrushaoAwaiting requested review from junrushao
@merrymercymerrymercyAwaiting requested review from merrymercy
@tqchentqchenAwaiting requested review from tqchen
@yzhliuyzhliuAwaiting requested review from yzhliu
@zhiicszhiicsAwaiting requested review from zhiics

+1 more reviewer

@jacobbohlinjacobbohlinjacobbohlin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@mbaret@Mousius@jacobbohlin@manupak