Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Set up graph_query testing infrastructure for task graph by psalz · Pull Request #325 · celerity/celerity-runtime · GitHub
Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Set up graph_query testing infrastructure for task graph by psalz · Pull Request #325 · celerity/celerity-runtime · GitHub
Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

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

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Set up graph_query testing infrastructure for task graph by psalz · Pull Request #325 · celerity/celerity-runtime · GitHub
Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Set up graph_query testing infrastructure for task graph by psalz · Pull Request #325 · celerity/celerity-runtime · GitHub
Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Set up graph_query testing infrastructure for task graph by psalz · Pull Request #325 · celerity/celerity-runtime · GitHub
Skip to content

Set up graph_query testing infrastructure for task graph - #325

Open
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure
Open

Set up graph_query testing infrastructure for task graph#325
psalz wants to merge 3 commits into
masterfrom
tdag-testing-infrastructure

Conversation

@psalz

Copy link
Copy Markdown
Member

This brings the TDAG (mostly) in line with CDAG and IDAG. What's missing is a hierarchical structure for task_record, but since unlike for commands and instructions, a call to submit() only creates a single task (except for horizons) it turns out we don't really need this functionality anyway.

@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (000e2892abb21ddd1ae413a5c86d7d95)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️Clang-Tidy found issue(s) with the introduced code (1/1)

Comment threadsrc/print_graph.cc Outdated
fmt::format_to(std::back_inserter(dot), "{}->{}[{}];", d.node, tsk.tid, dependency_style(d.kind, d.origin));
}
for(const auto& tsk : recorder.get_graph_nodes()) {
const char* shape = tsk->type == task_type::epoch || tsk->type == task_type::horizon ? "ellipse" : "box style=rounded";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️misc-const-correctness⚠️
variable shape of type const char * can be declared const

template <int Dims, typename Handler, typename Functor>
void dispatch_get_access(test_utils::mock_buffer<Dims>& mb, Handler& handler, access_mode mode, Functor rmfn) {
template <typename Builder, int Dims, typename Functor>
auto dispatch_get_access(Builder&& builder, test_utils::mock_buffer<Dims>& mb, access_mode mode, Functor rmfn) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️cppcoreguidelines-missing-std-forward⚠️
forwarding reference parameter builder is never forwarded inside the function body

Comment threadtest/task_graph_tests.cc Outdated

[[maybe_unused]] const task_id tid_8 =
test_utils::add_host_task(tt.tm, on_master_node, [&](handler& cgh) { buf_b.get_access<access_mode::read_write>(cgh, fixed<1>({0, 128})); });
const auto tid_8 = tctx.master_node_host_task().read_write(buf_b, fixed<1>({0, 128})).submit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️clang-analyzer-deadcode.DeadStores⚠️
Value stored to tid_8 during its initialization is never read

@coveralls

coveralls commented Dec 23, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 15470784509

Details

  • 33 of 33(100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on tdag-testing-infrastructure at 95.05%

TotalsCoverage Status
Change from base Build 15412343516:95.1%
Covered Lines:7146
Relevant Lines:7250

💛 - Coveralls

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

Very cool! This makes me want to start the TDAG refactor.

Comment on lines +18 to +19
// TODO: Can we make this the base class of cdag / idag test contexts?
class tdag_test_context final : private task_manager::delegate {

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.

How complicated would it actually be to have a common (interface) class for all four of these? We're copy pasting void device_compute() etc a lot. I'm not too worried about this since it's just test code, but would be neat to DRY it up if it's simple enough. I remember having to repeat the same test_context refactoring three times when I changed the fence surface API.

@GagaLP
GagaLPforce-pushed the tdag-testing-infrastructure branch from ab1c78b to 229761dCompareJune 5, 2025 15:15
@github-actions

Copy link
Copy Markdown

Check-perf-impact results: (17d2daea565336edd0421cacd8138ced)

❓ No new benchmark data submitted. ❓
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@psalz@coveralls@fknorr@GagaLP