-
Notifications
You must be signed in to change notification settings - Fork 1
Molecule Energy Simulator #131
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: develop
Are you sure you want to change the base?
Conversation
Probably fails checks because of incompatible python package versions? |
This demonstrator will compute the ground state energy for a given molecule using VQE algorithm . The molecule input is given in XYZ Format. |
Confirmed this, our current dwave version is incompatible with the qiskit version needed for this demonstrator. It's non trivial to solve this issue, so we might have to do the move to venv's now. |
Trying out the Copilot review feature :) |
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.
PR Overview
This PR introduces a new demonstrator called "Molecule Energy Simulator" for computing molecular ground state energies using a VQE algorithm and updates related configuration and supporting files.
- Added a new Java class (MoleculeEnergySimulator) that wires the Python simulation process into the application.
- Introduced a Python script (molecule-energy.py) that implements the simulation using Qiskit libraries.
- Updated demonstrator configuration and adjusted property names in CplexMipDemonstrator.
Reviewed Changes
File | Description |
---|---|
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java | Implements the new demonstrator using a Python process runner. |
demonstrators/qiskit/molecule-energy/molecule-energy.py | Provides the Python simulation code using Qiskit VQE methodology. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/DemonstratorConfiguration.java | Registers the new demonstrator with the problem manager. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/CplexMipDemonstrator.java | Adjusts the configuration property key for the Cplex MIP demonstrator. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 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.
PR Overview
This pull request introduces the new "Molecule Energy Simulator" demonstrator, which computes molecular ground state energies using a VQE algorithm and integrates a corresponding Python simulation script. Key changes include:
- The addition of the MoleculeEnergySimulator Java class for integrating a Python-based simulation.
- The creation of a new Python script (molecule-energy.py) that implements the simulation using Qiskit libraries.
- Updates to the demonstrator configuration and CplexMipDemonstrator to register and adjust properties for the new simulator.
Reviewed Changes
File | Description |
---|---|
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java | New demonstrator implementation for molecule energy simulation. |
demonstrators/qiskit/molecule-energy/molecule-energy.py | New Python simulation script with Qiskit functionalities; note duplicate import issue. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/DemonstratorConfiguration.java | Updated configuration to include the new demonstrator. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/CplexMipDemonstrator.java | Updated property key for Cplex demonstration. |
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java:60
- The new solve() method introduces production logic but there is no indication of accompanying test coverage; please consider adding tests to ensure its behavior.
public Mono<Solution<String>> solve(String input, SubRoutineResolver subRoutineResolver, SolvingProperties properties) {
Co-authored-by: Copilot <[email protected]>
37e6073
to
2409ecf
Compare
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.
Pull Request Overview
This PR introduces a new Molecule Energy Simulator demonstrator and its associated Python simulation script for molecule ground state energy computations.
- Added a new Java class (MoleculeEnergySimulator) that implements a demonstrator using VQE for energy simulation.
- Updated DemonstratorConfiguration to register the new simulator.
- Included a Python script (molecule-energy.py) for running molecule energy simulations with Qiskit.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java | New demonstrator implementation using VQE via a Python process. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/DemonstratorConfiguration.java | Updated configuration to register the new MoleculeEnergySimulator bean. |
demonstrators/qiskit/molecule-energy/molecule-energy.py | Added Python simulation script for molecule energy calculations with multiple simulation blocks. |
Files not reviewed (2)
- demonstrators/qiskit/molecule-energy/requirements.txt: Language not supported
- src/main/resources/application.properties: Language not supported
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.
Pull Request Overview
This PR introduces the "Molecule Energy Simulator" demonstrator, which integrates a new Java class with a Python-backed Qiskit implementation for computing molecular ground state energies using the VQE algorithm.
- Added MoleculeEnergySimulator.java implementing the demonstrator interface.
- Updated DemonstratorConfiguration.java to register the new demonstrator bean.
- Added molecule-energy.py, a Python script using Qiskit Nature libraries to compute and benchmark molecular energies.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java | New demonstrator implementation for simulating molecule energy |
src/main/java/edu/kit/provideq/toolbox/demonstrators/DemonstratorConfiguration.java | Updated configuration to include the new demonstrator |
demonstrators/qiskit/molecule-energy/molecule-energy.py | Python implementation for molecular energy simulation and experimental benchmarks |
Files not reviewed (2)
- demonstrators/qiskit/molecule-energy/requirements.txt: Language not supported
- src/main/resources/application.properties: Language not supported
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.
Pull Request Overview
This PR adds a new Molecule Energy Simulator demonstrator and integrates it into the existing framework while also standardizing error messages in several Python scripts.
- Introduces a new Java demonstrator (MoleculeEnergySimulator.java) that leverages a Python process to compute molecule energies via VQE.
- Updates DemonstratorConfiguration.java to register the new demonstrator.
- Standardizes exception types in multiple Python scripts and adds a new Qiskit-based molecule energy simulation script with visualization.
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java | Implements the new Molecule Energy Simulator demonstrator using VQE. |
src/main/java/edu/kit/provideq/toolbox/demonstrators/DemonstratorConfiguration.java | Registers the new demonstrator in the configuration. |
solvers/qiskit/qubo/qubo_qiskit.py | Changes the raised exception from TypeError to ValueError for argument validation. |
solvers/qiskit/max-cut/maxCut_qiskit.py | Updates the commented-out error message to use ValueError. |
solvers/custom/hs-knapsack/knapsack.py | Updates the raised exception from TypeError to ValueError for consistency. |
demonstrators/qiskit/molecule-energy/molecule-energy.py | Adds a new Qiskit-based simulation script with multiple simulation configurations and visual output. |
Files not reviewed (2)
- demonstrators/qiskit/molecule-energy/requirements.txt: Language not supported
- src/main/resources/application.properties: Language not supported
Comments suppressed due to low confidence (2)
src/main/java/edu/kit/provideq/toolbox/demonstrators/MoleculeEnergySimulator.java:56
- Consider enhancing the help text to include a concrete example of the expected XYZ format for clarity.
new TextSetting(false, SETTING_MOLECULE, "The molecule to be simulated in XYZ format - a di-hydrogen molecule by default", DEFAULT_MOLECULE)
demonstrators/qiskit/molecule-energy/molecule-energy.py:44
- Using two different mappers (ParityMapper earlier and JordanWignerMapper here) in a single script may be confusing. Consider adding a comment to explain the rationale behind using distinct mappers for different parts of the simulation.
from qiskit_nature.second_q.mappers import JordanWignerMapper
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.
looks very good, just minor comments. Adding a test or two would be quite helpful as well.
7e0e67f
to
969437b
Compare
|
||
var processResult = context | ||
.getBean(PythonProcessRunner.class, scriptPath, venv) | ||
.withArguments(molecule) |
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.
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.
Thanks for checking, I forgot to finish this PR :)
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.
@zaibod We currently just get a timeout in the pipeline. I can't really get my WSL version to run it on linux, can you check if it works on your side?
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.
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.
Seeing as we just read the output of the python script here, I guess we can also remove all of the text above, unless you think any of it would be useful?
969437b
to
5e3cb4b
Compare
|
Needs descriptions still