-
Notifications
You must be signed in to change notification settings - Fork 5
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
Implemented atomic representation for a single element type and valence electron only representation #39
Merged
Merged
Implemented atomic representation for a single element type and valence electron only representation #39
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
8e4956c
implemented single atom type rep generation
YAY-C a348d8e
Implemented valence-onyl atom density rep
YAY-C 9c3d5cd
Fixed argument passing
YAY-C 99c1eae
Refix argument-passing bug in 5653500
YAY-C e04664b
Merge branch 'master' into update_rho-spahm
YAY-C ae7a050
Add test for valence-only and single-element representations
YAY-C 0447f40
Fix broken-test (forgot to push test-data)
YAY-C b0801b2
Change default value of only_z from [] to None
briling 2503547
Add assert() for shape
YAY-C c236e07
Merge branch 'master' into update_rho-spahm
YAY-C 811d0bf
Add ECP argument to CLI
YAY-C 9531f3c
Revert "Fixed argument passing"
YAY-C de9e8c6
Revert "Implemented valence-onyl atom density rep"
YAY-C 4216077
Revert (patched) "Add test for valence-only and single-element repres…
YAY-C 4f0d593
Fix `only_z` default-value to None in CLI (b0801b2)
YAY-C bf9b393
Cleanup unnecessary data
YAY-C 8dc1deb
Revert "Add ECP argument to CLI"
YAY-C dc7981e
Made one-liner for reps packing into array
YAY-C File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
maybe this
can be replaced with this
which is easier to read and more similar to the previous version
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.
maybe even a one liner
but not sure about readability
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.
I liked the one line. but it doesn't work cause
mol.elements
is alist
and not anumpy.array
so you can not subscript it with a list of integers.the intermediate option does not work neither then!
I can create a copy of
mol.elements
in anumpy.array
and go for the oneliner, but it feels a bit like an overhead.what do you think?
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.
i'd put
zip(np.array(mol.elements)[only_i], rep)]
lolThere 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.
I just don't like j,i
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.
ahah sure! Done 👌