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

Throw error real-QGTOnTheFly@complexvector #885

Merged
merged 8 commits into from Jan 11, 2022
Merged

Throw error real-QGTOnTheFly@complexvector #885

merged 8 commits into from Jan 11, 2022

Conversation

PhilipVinc
Copy link
Member

@PhilipVinc PhilipVinc commented Aug 24, 2021

Check that if the qgt is real, the vector must be real too (currently this throws an untelligible error for QGTJacobianPyTree and returns the wrong result for OnTheFly).

@PhilipVinc PhilipVinc marked this pull request as draft August 24, 2021 10:54
@github-actions
Copy link

Hello and thanks for your Contribution!
I will be building previews of the updated documentation at the following link:
https://netket.github.io/netket/preview/qgt-error

Once the PR is closed or merged, the preview will be automatically deleted.

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2021

Codecov Report

Merging #885 (c0c1226) into master (30d938e) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #885      +/-   ##
==========================================
+ Coverage   81.17%   81.21%   +0.04%     
==========================================
  Files         203      204       +1     
  Lines       11696    11716      +20     
  Branches     1798     1800       +2     
==========================================
+ Hits         9494     9515      +21     
+ Misses       1755     1754       -1     
  Partials      447      447              
Impacted Files Coverage Δ
netket/optimizer/qgt/common.py 100.00% <100.00%> (ø)
netket/optimizer/qgt/qgt_jacobian_pytree.py 92.39% <100.00%> (+0.25%) ⬆️
netket/optimizer/qgt/qgt_onthefly.py 82.71% <100.00%> (+0.66%) ⬆️
netket/vqs/mc/mc_state/state.py 88.98% <0.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30d938e...c0c1226. Read the comment docs.

@PhilipVinc PhilipVinc marked this pull request as ready for review January 10, 2022 15:52
@PhilipVinc
Copy link
Member Author

this is now ready for review

Copy link
Collaborator

@femtobit femtobit 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. One remark on the changelog.

CHANGELOG.md Outdated
* Fix bug that prevented gradients of non-hermitian operators to be computed. The feature is still marked as experimental but will now run (we do not guarantee that results are correct). [#1045](https://github.com/netket/netket/pull/1045)
* Fix bug that prevented gradients of non-hermitian operators to be computed. The feature is still marked as experimental but will now run (we do not guarantee that results are correct). [#1053](https://github.com/netket/netket/pull/1053)
* Common lattice constructors such as `Honeycomb` now accepts the same keyword arguments as `Lattice`. [#1046](https://github.com/netket/netket/pull/1046)
* A typo in the diagonal of the RK4 Butcher Tableau has been fixed. This typo did not cause any numerical error, but the tableau was wrong nethertheless. [#1057](https://github.com/netket/netket/pull/1057)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not convinced this needs a changelog entry - it has no user-facing effect and the tableaus are in an internal module anyways.

@PhilipVinc PhilipVinc merged commit da6171c into master Jan 11, 2022
@PhilipVinc PhilipVinc deleted the qgt-error branch January 11, 2022 11:08
jwnys pushed a commit to jwnys/netket that referenced this pull request Jan 20, 2022
Check that if the qgt is real, the vector must be real too (currently this throws an untelligible error for QGTJacobianPyTree and returns the wrong result for OnTheFly).
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.

None yet

3 participants