Skip to content

added enr_ feature #7

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

added enr_ feature #7

wants to merge 5 commits into from

Conversation

GyunghunK
Copy link

Added the energy restricted space (enr) feature to jaxquantum. The code structure follows the QuTip implementation of the same feature.

@GyunghunK GyunghunK marked this pull request as draft November 7, 2024 22:38
@GyunghunK GyunghunK marked this pull request as ready for review December 31, 2024 23:25
@Phionx
Copy link
Contributor

Phionx commented Jan 1, 2025

Thanks for the PR @GyunghunK ! Please pull the new changes from the main branch in this repo to your branch, so that this can be merged in smoothly. Then, you will not be any number of commits behind EQuS/jaxquantum:main, as shown in the screenshot. You can do this manually via the git CLI or you can just press the "3 commits behind" text in the screenshot and it will automatically create a PR that pulls in recent changes from EQuS/jaxquantum:main into your branch. Let me know if you have any questions on how to do that.

Screenshot 2024-12-31 at 20 49 55

@GyunghunK
Copy link
Author

Happy new year Shantanu! I have merged the main branch to this one. Please check it out!

@Phionx
Copy link
Contributor

Phionx commented Jan 3, 2025

Great @GyunghunK , thanks for the quick update and happy new year to you too!

The code itself looks good to me.

Before I merge this, could you please add to the docs in two spots?

  1. Motivation and example usage of enr: https://github.com/EQuS/jaxquantum/blob/main/docs/advanced/enr.md
    Please expand on this computational technique a bit more than in the jupyter notebook so that it's clear when the ENR basis is worth using. If you can, a runtime or memory simulation would be awesome (and very useful!).

  2. Docs on how to run tests:
    https://github.com/EQuS/jaxquantum/blob/main/docs/getting_started/tests.md
    Please add a note on how to locally run the tests you added.

You may have to merge main into your branch again before you see these files.

@Phionx
Copy link
Contributor

Phionx commented May 6, 2025

Hey @GyunghunK , can you please merge main into this branch and check if your tests still pass? If so, please let me know and I'll merge it. If you can add the motivating example I mentioned in my last comment, that would be great. Even if not, I'd like to merge this in and close this PR. Thanks!

Happy new year Shantanu! I have merged the most recent main branch to feature/enr!
@GyunghunK
Copy link
Author

Sorry for the delay. I have merged the main and checked the notebook and tests regarding the enr submodule. Thank you!

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.

2 participants