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

GCNN Bug Fixes #1428

Merged
merged 6 commits into from Feb 12, 2023
Merged

GCNN Bug Fixes #1428

merged 6 commits into from Feb 12, 2023

Conversation

chrisrothUT
Copy link
Collaborator

@chrisrothUT chrisrothUT commented Feb 8, 2023

This PR fixes the bugs described in #1405.

Fix #1405

@chrisrothUT chrisrothUT changed the title Bug Fixes GCNN Bug Fixes Feb 8, 2023
@codecov
Copy link

codecov bot commented Feb 8, 2023

Codecov Report

Merging #1428 (93042db) into master (4899182) will increase coverage by 0.24%.
The diff coverage is 100.00%.

❗ Current head 93042db differs from pull request most recent head 3fad745. Consider uploading reports for the commit 3fad745 to get more accurate results

@@            Coverage Diff             @@
##           master    #1428      +/-   ##
==========================================
+ Coverage   83.30%   83.55%   +0.24%     
==========================================
  Files         233      229       -4     
  Lines       13382    12977     -405     
  Branches     2028     1961      -67     
==========================================
- Hits        11148    10843     -305     
+ Misses       1715     1635      -80     
+ Partials      519      499      -20     
Impacted Files Coverage Δ
netket/models/equivariant.py 86.69% <ø> (ø)
netket/nn/symmetric_linear.py 87.53% <100.00%> (+4.04%) ⬆️
netket/utils/array.py 100.00% <100.00%> (ø)
netket/optimizer/__init__.py 61.90% <0.00%> (-9.53%) ⬇️
netket/stats/mc_stats_old.py 90.41% <0.00%> (-2.74%) ⬇️
netket/operator/_local_operator_compile_helpers.py 70.00% <0.00%> (-2.00%) ⬇️
netket/logging/json_log.py 72.36% <0.00%> (-1.32%) ⬇️
netket/operator/boson.py 73.33% <0.00%> (-0.96%) ⬇️
netket/vqs/base.py 81.13% <0.00%> (-0.95%) ⬇️
netket/driver/abstract_variational_driver.py 95.45% <0.00%> (-0.91%) ⬇️
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@PhilipVinc PhilipVinc self-requested a review February 9, 2023 08:30
@PhilipVinc
Copy link
Member

PhilipVinc commented Feb 9, 2023 via email

@PhilipVinc
Copy link
Member

Ok, I added the tests for DenseSymm (and found out there were a few bugs, and fixed em), can you add the ones for GCNN to be sure there are no bugs?

@PhilipVinc
Copy link
Member

@chrisrothUT I would like to tag a new release of netket and the last thing that is missing is this bug fix... Can I ask you to add a test that would have failed before this fix for the GCNN (so checking that the masks are specified?)

@chrisrothUT
Copy link
Collaborator Author

Okay I added a test to make sure the masks are being used correctly

@PhilipVinc
Copy link
Member

Lovely! Thanks a lot! When tests pass on CI I'll merge.

Early next week I'll tag a new release of netket...

@PhilipVinc
Copy link
Member

Thank you!

@PhilipVinc PhilipVinc merged commit 6b050bf into netket:master Feb 12, 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.

Passing masks to a GCNN has no effect
2 participants