-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor(structure): restructure code so we can properly support many… #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Independence Tests and Orientations Tests are relabelled as Pipeline Steps which makes the code more readable. Relicts are removed and the file structure is refined. Only some doc strings should be formulated differently for clarity.
|
||
class AbstractGraphModel(GraphModelInterface, ABC): | ||
""" | ||
The graph model is the main class of causy. It is responsible for creating a graph from data and executing the pipeline_steps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean: The graph model is the main class of causy. It is responsible for initialising nodes for each variable that we have data from, then initializing a fully connected graph by adding edges between each pair of these nodes and then executing the pipeline_steps.
The graph model is the main class of causy. It is responsible for creating a graph from data and executing the pipeline_steps. | ||
|
||
The graph model is responsible for the following tasks: | ||
- Create a graph from data (create_graph_from_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"create nodes for each variabel" or "initialise a fully connected graph"
""" | ||
Take the actions returned by the test | ||
|
||
In causy changes on the graph are not executed directly. Instead, the test returns an action which should be executed on the graph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test -> independence test or orientation rule
Take the actions returned by the test | ||
|
||
In causy changes on the graph are not executed directly. Instead, the test returns an action which should be executed on the graph. | ||
This is done to make it possible to execute the tests in parallel as well as to decide proactively at which point in the decisions taken by the pipeline step should be executed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"at which part in the decision": minus in I think
|
||
In causy changes on the graph are not executed directly. Instead, the test returns an action which should be executed on the graph. | ||
This is done to make it possible to execute the tests in parallel as well as to decide proactively at which point in the decisions taken by the pipeline step should be executed. | ||
Actions are returned by the test and are executed on the graph. The actions are stored in the action history to make it possible to revert the actions or use them in a later step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test -> pipeline step? independence test or orientation rule? (it's not only tests)
This is done to make it possible to execute the tests in parallel as well as to decide proactively at which point in the decisions taken by the pipeline step should be executed. | ||
Actions are returned by the test and are executed on the graph. The actions are stored in the action history to make it possible to revert the actions or use them in a later step. | ||
|
||
:param results: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the results of the tests and rules
Actions are returned by the test and are executed on the graph. The actions are stored in the action history to make it possible to revert the actions or use them in a later step. | ||
|
||
:param results: | ||
:return: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the action on the graph
|
||
def execute_pipeline_step(self, test_fn: PipelineStepInterface): | ||
""" | ||
Execute a single pipeline_step on the graph. either in parallel or in a single process depending on the test_fn.parallel flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either -> Either
… algorithms