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

Restructure the files under algorithms.gradients. #9695

Closed
a-matsuo opened this issue Mar 1, 2023 · 2 comments · Fixed by #9969
Closed

Restructure the files under algorithms.gradients. #9695

a-matsuo opened this issue Mar 1, 2023 · 2 comments · Fixed by #9969
Labels
mod: algorithms Related to the Algorithms module type: feature request New feature or request

Comments

@a-matsuo
Copy link
Contributor

a-matsuo commented Mar 1, 2023

What should we add?

Currently the files under under algorithms.gradients are messy. It's better to restructure the files. Grouping files by gradient type seems like a good idea. The files related to qgt and qfi are a bit tricky though.

gradients/
├──base/
│   ├── base_estimator_gradient.py
│   └── base_sampler_gradient.py
├── finite_diff/
│   ├── finite_diff_estimator_gradient.py
│   └── finite_diff_sampler_gradient.py
├── lin_comb/
│   ├── lin_comb_estimator_gradient.py
│   └── lin_comb_sampler_gradient.py
├── param_shift/
│   ├── param_shift_estimator_gradient.py
│   └── param_shift_sampler_gradient.py
├── qgt/
│   ├── base_qgt.py
│   ├── lin_comb_qgt.py
│   └── reverse_qgt.py
├── qfi/
│   └── qfi.py
└── reverse/
    └── reverse_gradient.py
@a-matsuo a-matsuo added the type: feature request New feature or request label Mar 1, 2023
@woodsp-ibm
Copy link
Member

Is this just a code restructuring that will be transparent to end users? I.e. all the function will be imported into the gradient init file and an end user will still import from algorithms.gradients.

In the gradients in the docstring the organization/presentation to the user is not the same as the above structure (has Estimator grads, Sampler grads and QGT) - would you change that too, to reflect this organization or not. It would not affect the code in any way just how they are grouped in the docs.

@woodsp-ibm woodsp-ibm added the mod: algorithms Related to the Algorithms module label Mar 1, 2023
@a-matsuo
Copy link
Contributor Author

a-matsuo commented Mar 2, 2023

Is this just a code restructuring that will be transparent to end users? I.e. all the function will be imported into the gradient init file and an end user will still import from algorithms.gradients.

Yes! exactly.

In the gradients in the docstring the organization/presentation to the user is not the same as the above structure (has Estimator grads, Sampler grads and QGT) - would you change that too, to reflect this organization or not. It would not affect the code in any way just how they are grouped in the docs.

I hadn't thought of that. I think it's better to group by gradient type in the docs, too, if the file structures are so.

github-merge-queue bot pushed a commit that referenced this issue Jun 28, 2023
* Reorganised algorithms.gradients

Fixes issue #9695 by reorganising algorithms.gradients into seperate folders.

* Moved 'lin_comb_qgt.py' to the 'qgt' folder

Moved 'lin_comb_qgt.py' to the 'qgt' folder to ensure all qgt related files are grouped together.

* Formatted files with linter.

Ran linter and reformatted files accordingly.

* Updated '__init__.py'

Updated '__init__.py' to reflect new folder structure.

* Removed 'qgt' folder

Assimilated QGT classes into related folders within 'algorithms.gradients'. Also, 'qgt_result.py' was placed into the 'base' folder since it constitutes a base class.

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Removed 'qfi' folder

Removed 'qfi' folder and moved remaining contents into parent folder 'gradients'.

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Attempted to resolve merge conflicts.

Altered files to match new versions causing the conflict.

* Ran linter

Ran 'tox -eblack' on the repository to resolve linter issues.

* Fixed base module hierarchy issue

Altered contents of 'qfi.py' to refer to 'BaseQGT' according to the new hierarchy given in the PR.

* Fixed more hierarchy issues.

Changed more references to reflect new hierarchy.

---------

Co-authored-by: Julien Gacon <[email protected]>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this issue Jul 17, 2023
* Reorganised algorithms.gradients

Fixes issue Qiskit/qiskit#9695 by reorganising algorithms.gradients into seperate folders.

* Moved 'lin_comb_qgt.py' to the 'qgt' folder

Moved 'lin_comb_qgt.py' to the 'qgt' folder to ensure all qgt related files are grouped together.

* Formatted files with linter.

Ran linter and reformatted files accordingly.

* Updated '__init__.py'

Updated '__init__.py' to reflect new folder structure.

* Removed 'qgt' folder

Assimilated QGT classes into related folders within 'algorithms.gradients'. Also, 'qgt_result.py' was placed into the 'base' folder since it constitutes a base class.

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Removed 'qfi' folder

Removed 'qfi' folder and moved remaining contents into parent folder 'gradients'.

* Update qiskit/algorithms/gradients/__init__.py

Co-authored-by: Julien Gacon <[email protected]>

* Attempted to resolve merge conflicts.

Altered files to match new versions causing the conflict.

* Ran linter

Ran 'tox -eblack' on the repository to resolve linter issues.

* Fixed base module hierarchy issue

Altered contents of 'qfi.py' to refer to 'BaseQGT' according to the new hierarchy given in the PR.

* Fixed more hierarchy issues.

Changed more references to reflect new hierarchy.

---------

Co-authored-by: Julien Gacon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: algorithms Related to the Algorithms module type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants