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

Expose result of convergence of LOM analysis' Q3D renderer to end user #915

Conversation

jagandecapri
Copy link
Contributor

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly.
✅ I have read the CONTRIBUTING document.

What are the issues this pull addresses (issue numbers / links)?

Fixes #914

Did you add tests to cover your changes (yes/no)?

Yes

Did you update the documentation accordingly (yes/no)?

Yes

Did you read the CONTRIBUTING document (yes/no)?

Yes

Summary

Currently, the result of convergence of LOM Analysis' Q3D renderer is not exposed to the end user.
Exposing the result of convergence to the user would give feedback to the user on whether the results obtained is reliable. Users can also programatically increase the number of passes and rerun the simulation to achieve convergence.

Details and comments

The PR only covers Q3D renderer at the moment.

Convergence data is extracted from q3d simulation. Implementation is
copied from Sweeping class
is_converged property is added to LumpedElementsSim class
Add 2 tests to get convergence boolean.
1. Test to get True when simulation converge
2. Test to get False when simulation did not converge
is_converged variable is added to the data labels
The notebook is executed to illustrate the usage of is_converged
key. Example is shown in the notebook with accompanying textual
explanation.
The notebook is updated to illustrate the use of is_converged
key. Code example accompanying textual explanation is added to the
notebook. The notebook is not executed.
Copy link
Collaborator

@priti-ashvin-shah-ibm priti-ashvin-shah-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super, I like the recycling of code from sweeping class to sweeper class.

Copy link
Collaborator

@obrienpja obrienpja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

Copy link
Collaborator

@ThomasGM4 ThomasGM4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@priti-ashvin-shah-ibm
Copy link
Collaborator

Will merge for you.

@priti-ashvin-shah-ibm priti-ashvin-shah-ibm merged commit eaf9162 into qiskit-community:main Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose result of convergence of LOM analysis' Q3D renderer to end user
4 participants