Add Effective Dimension class #364

ElePT · 2022-04-04T13:24:35Z

Summary

This PR closes #340 by adding the LocalEffectiveDimensionClass introduced in https://arxiv.org/abs/2112.04807, taking the original experiments' code as reference: https://github.com/amyami187/effective_dimension.

Details and comments

The local effective dimension shares all the logic from the global effective dimension algorithm, with the only difference being the limitation in number of parameter sets used. However, it has been discussed to be conceptually different enough to become its own class.

Please let me know if you disagree with any variable names.

qiskit_machine_learning/neural_networks/effective_dimension.py

woodsp-ibm · 2022-04-19T19:23:59Z

qiskit_machine_learning/neural_networks/effective_dimension.py

+        weight_samples: Union[List[float], np.ndarray, int] = 1,
+        input_samples: Union[List[float], np.ndarray, int] = 1,


This I brought up before, but I am not sure its resolved. The type hint here says List[float] but should it be List[List[float]] since its really a two dimensional affair i.e. An array of neural network parameters (weights), of shape (num_weight_samples, num_weights), or an int. It could be reshaped from a flat list but I did not see that.

What I see in the setter for instance is this

len(weight_samples.shape) > 2 or weight_samples.shape[1] is not self._model.num_weights

That checks the dimensions (size of the shape) being > 2 but if 1 will end up failing on the next line as it will index shape[1] which will be out of bounds for a flat list.

I suggested adding unit tests to ensure it ran with plain lists as expected being passed in to make sure of things.

While you are absolutely right, I'm not sure I want to keep List in type hints at all. They tend to cause many problems, especially when multidimensional arrays are required. I'd prefer to remove support for List completely in favor of plain np.ndarray. I think I know where this type hint originates from. There's the same problem in NeuralNetwork, same list type hint and it is also incorrect, unfortunately. In NeuralNetwork a multidimensional array is expected. Usually it is of shape (num_samples, num_features).

Therefore, input_samples may be a single value, a plain list or a list of list at least. I can extend type hints, but prefer to remove support for lists and this is a good chance to start it off. There's an old issue to consider as well: #159.

OK, I removed lists from the type hints.

test/neural_networks/test_effective_dimension.py

woodsp-ibm · 2022-04-19T19:29:34Z

test/neural_networks/test_effective_dimension.py

+        effdim1 = global_ed1.get_effective_dimension(self.n)
+        effdim2 = global_ed2.get_effective_dimension(self.n)
+
+        self.assertAlmostEqual(effdim1, effdim2, 5)


This asserts two results are the same - in my mind it would be good to have an expected reference value at least one of these was compared against to ensure that they both don't go bad together and we never notice. I see this is done in other tests too, but I will just comment here.

Added an explicit assert.

woodsp-ibm · 2022-04-19T19:30:06Z

test/neural_networks/test_effective_dimension.py

+        print(effdim1)
+        print(effdim2)


We should not leave print statements from debugging in the code

Yes, I forgot to remove prints while was working on tests.

qiskit_machine_learning/neural_networks/effective_dimension.py

woodsp-ibm · 2022-04-19T20:15:21Z

qiskit_machine_learning/neural_networks/effective_dimension.py

+        # input setter uses self._model
+        self.input_samples = input_samples  # type: ignore
+
+    def num_weights(self) -> int:


This ends up being a method i.e. you would call it as ed.num_weights() versus accessing it as property on the neural network i.e. self._model.num_weights Would we prefer a property getter here if this functionality is needed?

I'm not sure it is required. I removed this method.

woodsp-ibm · 2022-04-19T20:25:10Z

qiskit_machine_learning/neural_networks/effective_dimension.py

+        """Returns the dimension of the model according to the definition from [1]."""
+        return self._model.num_weights


Returns the dimension of the model... So far we have given a NeuralNetwork as a qnn into the constructor. Yes it was stored as _model but somehow I imagine the term model to be something I have to read the reference and its not something we refer qnn to be in the constructor docstring. This is also call num_weights not dim or dimension. I think maybe, something more like this perhaps
Returns the number of weights of the supplied neural network. This is the dimension of the model according to the definition from [1]. where we explain it in terms of the code here and state the correspondence to the paper rather than the latter that being the main thing. Since we passed in qnn and ought to know num_weights from that, this indirect access to it - is it really useful?

I believe num_weights can be easily retrieved from qnn, so I don't see a real need for this method.

woodsp-ibm · 2022-04-19T20:28:28Z

qiskit_machine_learning/neural_networks/effective_dimension.py

+
+    @weight_samples.setter
+    def weight_samples(self, weight_samples: Union[List[float], np.ndarray, int]) -> None:
+        """Sets network parameters (weights)."""


Since the input samples setter/getter is more direct and says Sets network input samples. Would this and the getter above be more direct as Sets network weights samples. - why do we need to use the term parameters here and then have to qualify it with (weights) in order for it to presumably be understood in context here.

woodsp-ibm · 2022-04-19T20:29:48Z

qiskit_machine_learning/neural_networks/effective_dimension.py

+            weight_samples = np.asarray(weight_samples)
+            if (
+                len(weight_samples.shape) > 2
+                or weight_samples.shape[1] is not self._model.num_weights


is not - do we mean != here

Oh, good point, of course it must be !=.

Co-authored-by: Steve Wood <[email protected]>

qiskit_machine_learning/neural_networks/effective_dimension.py

Co-authored-by: Steve Wood <[email protected]>

* Add eff dim class and demo * Refactor get_fisher * Refactor get_fisher * Refactor docstrings * Update effdim examples * Add mock runtime code and demo * Add effdim runtime examples * Refactor variable names, remove loops * Add unit tests * add eff dim class * add unit tests with fixed random seed * Remove tutorial * Remove runtime code * Fix lint * Fix style * Fix date * Add reno * add reno for effective dim. * Fix mypy * For updating local branch * Remove unnecesary files * Restore files * Fix spelling and tipe hinting * Address style comments * Add sphinx class refs. * Reno effective dimension * Modify random seed * Update docstring Co-authored-by: Anton Dekusar <[email protected]> * Update docstring Co-authored-by: Anton Dekusar <[email protected]> * Address docstring comment (d) Co-authored-by: Anton Dekusar <[email protected]> * Apply suggestion Co-authored-by: Anton Dekusar <[email protected]> * Change method names * Add more verbose callback * Remove d (num_weights), add return shapes * Address getter/setter feedback * Fix local eff dim setter * Address unittest feedback * Fix formatting * Add references * Fix lint * Refactor docstrings * Add to pylintdict * Bugfix * Add expressibility * Fix callable type * Attempt to remove tutorial * Remove tutorial * Fix signature * Apply suggestion reference Co-authored-by: dlasecki <[email protected]> * Apply suggestion doctoring Co-authored-by: dlasecki <[email protected]> * Apply suggestion unpack Co-authored-by: dlasecki <[email protected]> * Update test/algorithms/test_effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update effective_dimension.py * For undoing commit * Revert "Apply suggestion doctoring" This reverts commit 4b15c48. * Revert unpack commit * Apply algorithm globals change * Add classes to init * Add check to local eff dim setter * Remove seed * Add class to init * Commit changes * Changes to test * Fix checks * Fix mypy * Remove local num params * Add classification example * Add classification example * Update qiskit_machine_learning/algorithms/__init__.py Co-authored-by: Steve Wood <[email protected]> * Change num_inputs, num_params * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update qiskit_machine_learning/algorithms/effective_dimension.py Co-authored-by: dlasecki <[email protected]> * Update test/algorithms/test_effective_dimension.py Co-authored-by: Anton Dekusar <[email protected]> * Adapt unit tests for num_inputs, num_params * Remove tutorial * Change location, add callback to local eff dim * Bugfix after applying suggestions * Fix style * Add ED to init docstring * Fix type assignment * Remove ED from algorithms init * Fix black * Add ED to init * Add Eff. Dim reno * Add input unit tests * Change vars names * Fix style * Modify callbacks, add input checks * Fix black * Add monte and carlo * Fix copyright * Modify param setter check * Fix style * Fix mypy * change namings * refactor tests, fix sphinx * more renamings * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * Update test/neural_networks/test_effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * code review * fix black * fix != * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> * remove lists from type hints * Update qiskit_machine_learning/neural_networks/effective_dimension.py Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Anton Dekusar <[email protected]> Co-authored-by: dlasecki <[email protected]> Co-authored-by: Amira Abbas <[email protected]> Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Anton Dekusar <[email protected]>

ElePT added 18 commits March 4, 2022 15:48

Add eff dim class and demo

aae3d8e

Refactor get_fisher

14047aa

Refactor get_fisher

6f6d715

Refactor docstrings

b0d5437

Update effdim examples

b98b3ae

Add mock runtime code and demo

4500a9c

Add effdim runtime examples

60cc4a5

Refactor variable names, remove loops

6ff06fc

Add unit tests

cceaa7e

add eff dim class

2c0b2b9

add unit tests with fixed random seed

4d1b4b1

Remove tutorial

c70a202

Remove runtime code

9cbd01a

Fix lint

e92428d

Fix style

59d4082

Fix date

6e4dd43

Add reno

b718b93

add reno for effective dim.

f7b67e3

ElePT requested review from stefan-woerner, pbark, manoelmarques, woodsp-ibm and adekusar-drl as code owners April 4, 2022 13:24

ElePT added the Changelog: New Feature Include in the Added section of the changelog label Apr 4, 2022

ElePT self-assigned this Apr 4, 2022

ElePT and others added 5 commits April 4, 2022 15:25

Merge branch 'main' into eff-dim-class

7257e2b

Fix mypy

6148aee

For updating local branch

409c4ad

Update local branch

c45200a

Remove unnecesary files

da4e1b9

woodsp-ibm reviewed Apr 19, 2022

View reviewed changes

qiskit_machine_learning/neural_networks/effective_dimension.py Outdated Show resolved Hide resolved

woodsp-ibm reviewed Apr 19, 2022

View reviewed changes

test/neural_networks/test_effective_dimension.py Outdated Show resolved Hide resolved

woodsp-ibm reviewed Apr 19, 2022

View reviewed changes

qiskit_machine_learning/neural_networks/effective_dimension.py Outdated Show resolved Hide resolved

woodsp-ibm reviewed Apr 19, 2022

View reviewed changes

qiskit_machine_learning/neural_networks/effective_dimension.py Outdated Show resolved Hide resolved

woodsp-ibm reviewed Apr 19, 2022

View reviewed changes

adekusar-drl and others added 9 commits April 19, 2022 23:43

Update qiskit_machine_learning/neural_networks/effective_dimension.py

9757860

Co-authored-by: Steve Wood <[email protected]>

Update qiskit_machine_learning/neural_networks/effective_dimension.py

bfa8e26

Co-authored-by: Steve Wood <[email protected]>

Update test/neural_networks/test_effective_dimension.py

fda07a0

Co-authored-by: Steve Wood <[email protected]>

Update qiskit_machine_learning/neural_networks/effective_dimension.py

88fae4f

Co-authored-by: Steve Wood <[email protected]>

Update qiskit_machine_learning/neural_networks/effective_dimension.py

c399fb5

Co-authored-by: Steve Wood <[email protected]>

code review

f628d95

Merge remote-tracking branch 'ElePT/eff-dim-class' into eff-dim-class

7771fd6

fix black

2dc4264

fix !=

c74cb4e

woodsp-ibm reviewed Apr 20, 2022

View reviewed changes

qiskit_machine_learning/neural_networks/effective_dimension.py Outdated Show resolved Hide resolved

woodsp-ibm reviewed Apr 20, 2022

View reviewed changes

qiskit_machine_learning/neural_networks/effective_dimension.py Outdated Show resolved Hide resolved

adekusar-drl and others added 5 commits April 20, 2022 14:09

Merge branch 'main' into eff-dim-class

70c46ed

Update qiskit_machine_learning/neural_networks/effective_dimension.py

1bca3be

Co-authored-by: Steve Wood <[email protected]>

remove lists from type hints

a6f2ac2

Update qiskit_machine_learning/neural_networks/effective_dimension.py

9dbb599

Co-authored-by: Steve Wood <[email protected]>

Merge remote-tracking branch 'ElePT/eff-dim-class' into eff-dim-class

0672248

woodsp-ibm approved these changes Apr 21, 2022

View reviewed changes

adekusar-drl merged commit 6a7c8c0 into qiskit-community:main Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Effective Dimension class #364

Add Effective Dimension class #364

ElePT commented Apr 4, 2022 •

edited

Loading

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

woodsp-ibm Apr 19, 2022

adekusar-drl Apr 20, 2022

		weight_samples: Union[List[float], np.ndarray, int] = 1,
		input_samples: Union[List[float], np.ndarray, int] = 1,

		"""Returns the dimension of the model according to the definition from [1]."""
		return self._model.num_weights

Add Effective Dimension class #364

Add Effective Dimension class #364

Conversation

ElePT commented Apr 4, 2022 • edited Loading

Summary

Details and comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElePT commented Apr 4, 2022 •

edited

Loading