Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial skeleton for flow_graph #4158

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

eric-hughes-tiledb
Copy link
Contributor

Initial skeleton for flow_graph. Focus is on (1) overall structure with decomposition into stages mediated by concepts, and (2) passage through the stages from a static graph specification to an executable graph.

Adds new directives to .clang-format to account for concept. This requires an upgrade to version 16.


TYPE: NO_HISTORY
DESC: Initial skeleton for flow_graph

… with decomposition into stages mediated by concepts, and (2) passage through the stages from a static graph specification to an executable graph.

Adds new directives to `.clang-format` to account for concept. This requires an upgrade to version 16.

* Specifying a node by means of its node body: `flow_graph/node.h`
* Specifying a flow graph: `flow_graph/graph.h`
* Creating and running a flow graph: `flow_graph/system.h`
Copy link
Contributor

Choose a reason for hiding this comment

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

What about general_factory.h?

The header `flow_graph/graph.h` contains support classes and all the concepts
need to specify a graph, either statically or dynamically.

### Flow graph execution objects
Copy link
Contributor

Choose a reason for hiding this comment

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

Something might need to be added here as well for general_factory.

@@ -0,0 +1,41 @@
/**
* @file flow_graph/node_body.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @file flow_graph/node_body.h
* @file flow_graph/graph.h

@@ -0,0 +1,25 @@
#
# flow_graph/execution_platform/CMakeLists.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

flow_graph/library/platform/CMakeLists.txt

@@ -0,0 +1,48 @@
/**
* @file flow_graph/library/basic/basic_execution_platform.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @file flow_graph/library/basic/basic_execution_platform.h
* @file tiledb/flow_graph/library/platform/minimal_execution_platform.h

}
}

//---------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove.


namespace tiledb::flow_graph {

//-------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: weird box type.

template <class T>
concept empty_product_type = product_type<T> && (factor_sizeof<T>() == 0);

//----------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the empty box for?

constexpr bool is_equal_reference(const T& x, const U& y) {
if constexpr (!(std::is_same_v<std::remove_cv_t<T>, std::remove_cv_t<U>>)) {
return false;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be doing what the comment is saying?

template <typename... U>
reference_tuple(const U&...) -> reference_tuple<const U&...>;

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove?

Sign up for free to 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.

None yet

2 participants