-
Notifications
You must be signed in to change notification settings - Fork 7
[WIP] Data based trainers #32
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
base: main
Are you sure you want to change the base?
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…ing_pipeline into database_trainers
AlbertoBaiardi
left a comment
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 modified the naming of some variables but this triggered more changes than expected :) Anyways, the rest look good to me! I only noted a few points where the documentation was not super-clear to me while going through the code.
|
|
||
|
|
||
| class BaseAngleAggregator(ABC): | ||
| """A base class that matches features from problem instances to 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.
Could you maybe extend the doc here? It is not super-clear to me what is meant by "data" -- does it mean "a given target problem"?
| For example, for example, multiple sets of angles might have been identified as | ||
| good QAOA angles for a given instance. We could then average over these angles or | ||
| try and identify trends in them to fit. |
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.
| For example, for example, multiple sets of angles might have been identified as | |
| good QAOA angles for a given instance. We could then average over these angles or | |
| try and identify trends in them to fit. | |
| For example, for example, multiple sets of angles might have been identified as | |
| good QAOA angles for a given instance. We could then average over these angles or | |
| try and extrapolate these data to new problem instances. |
(I tried extending the documentation -- double-check if I kept the original meaning :) )
| def __call__(self, qaoa_angles: List): | ||
| """Do not do any aggregation.""" | ||
| return qaoa_angles |
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.
Possibly stupid question: from the documentation above I understood that qaoa_angles contains a set of parameters obtained for multiple problem instances. Hence, I expected the qaoa_angles to be somehow "aggregated" before returning a value.
|
|
||
| def __call__(self, qaoa_angles: np.array): | ||
| """Average over the qaoa_angles.""" | ||
| return np.average(qaoa_angles, axis=self._axis) |
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.
| return np.average(qaoa_angles, axis=self._axis) | |
| if any(i >= len(qaoa_angles.shape) for i in self._axis): | |
| raise ValueError("Input data not coherent with chosen axes") | |
| return np.average(qaoa_angles, axis=self._axis) |
| num_nodes: bool = True, | ||
| num_edges: bool = True, | ||
| avg_node_degree: bool = True, | ||
| avg_edge_weights: bool = True, | ||
| standard_devs: bool = True, | ||
| density: bool = True, |
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.
| num_nodes: bool = True, | |
| num_edges: bool = True, | |
| avg_node_degree: bool = True, | |
| avg_edge_weights: bool = True, | |
| standard_devs: bool = True, | |
| density: bool = True, | |
| extract_num_nodes: bool = True, | |
| extract_num_edges: bool = True, | |
| extract_avg_node_degree: bool = True, | |
| extract_avg_edge_weights: bool = True, | |
| extract_standard_devs: bool = True, | |
| extract_density: bool = True, |
(I find these names clearer)
| def __call__(self, x: list) -> list: | ||
| """Compute the QAOA angles from the principal components.""" |
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.
Would x be a list of float, containing the angles, and the return would be of size self._num_components, correct? If so, I would maybe mention this in the docstring 🙂
| qaoa_angles_function: A function to convert optimization parameters into QAOA | ||
| angles. By default, this is the identity function. Ideally, this argument is | ||
| an instance of `BaseAnglesFunction` but we allow any callable here that maps | ||
| optimization parameters to QAOA angles. |
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.
Reverse? I.e., "QAOA angles to optimization parameters"?
| # values are QAOA angles corresponding to those features. The values may | ||
| # be a list. | ||
| self._data_loader = data_loader | ||
| self._data: dict = data_loader() |
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.
| self._data: dict = data_loader() | |
| self._data: dict = self._data_loader() |
| # values are QAOA angles corresponding to those features. The values may | ||
| # be a list. |
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.
Why "may be a list"? Shouldn't it be at least of size 2? (1 for gamma and 1 for beta)
| if self._evaluator is not None: | ||
| energy = self._evaluator.evaluate(cost_op, qaoa_angles) | ||
| else: | ||
| energy = "NA" |
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.
Is there a specific reason not to raise an error here?
| def test_simple(self): | ||
| """Test that the workflow runs.""" | ||
|
|
||
| data_loader = TrivialDataLoader(self._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.
| data_loader = TrivialDataLoader(self._data) | |
| data_loader = TrivialDataLoader(self._data_base) |
(I think this should make the CI happy)
Summary
This PR contributes (i) a PCA and (ii) a framework to transfer parameters based on data.
Details and comments
The notebooks in the PR are a good place to get started. The PR introduces small tools to load data, extract features, match them and aggregate angles. These tools are designed to be created from config files so that we can define method files for these methods. The PR also reworks the graph feature extraction.
TODOs
Version updated
Please increment the
qaoa_training_pipeline_versionvariable and extend the main README.md.