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

Nexus: Adding support for ghost atoms #1653

Merged
merged 2 commits into from
Jun 21, 2019
Merged

Conversation

shivupa
Copy link
Contributor

@shivupa shivupa commented Jun 20, 2019

Allows nexus to understand ghost atoms.

Why this is necessary:
The converters from quantum chemistry codes (e.g. gaussian/gamess) don't currently know about ghost atoms either, but they will parse the correct basis functions/MO coefficients. These just get assigned to atoms of type "". Replacing these with something reasonable like "X" and trying to uses nexus post calculation for analysis results in

PhysicalSystem error:
    particle X is unknown
  exiting.

This fixes the above error so now QmcpackAnalyzer can be used on systems with ghost atoms.

@qmc-robot
Copy link

Can one of the maintainers verify this patch?

@markdewing
Copy link
Contributor

okay to test

@jtkrogel
Copy link
Contributor

jtkrogel commented Jun 20, 2019

This looks like it should work, though permanently adding atom X to the periodic table is unusual.

Does the following work?

from physical_system import Matter,Ion
from qmcpack_analyzer import QmcpackAnalyzer

Matter.particle_collection.add_particles(Ion('X'))

qa = QmcpackAnalyzer('./your.in.xml')
qa.analyze()

@shivupa
Copy link
Contributor Author

shivupa commented Jun 20, 2019

Matter.particle_collection.add_particles(Ion('X',mass=0,charge=0,spin=0,protons=0,neutrons=0))

This does work. I could use this since I'm guessing ghost atoms aren't very commonly used.

@jtkrogel
Copy link
Contributor

jtkrogel commented Jun 20, 2019

I think having a way of supporting ghost atoms is a good idea.

One possibility is to add a function like this to physical_system.py

def ghost_atoms(*names):
    for name in names:
        Matter.particle_collection.add_particles(Ion(name,mass=0,charge=0,spin=0,protons=0,neutrons=0))
    #end for
#end def ghost_atoms

and use it outside

ghost_atoms('X')
qa = QmcpackAnalyzer(...)

or move its use into QmcpackAnalyzer instead

qa = QmcpackAnalyzer(...,ghost_atoms=['X'])

If having an atom X permanently available is more convenient, then I'm fine if you want to add this line

Matter.particle_collection.add_particles(Ion('X',mass=0,charge=0,spin=0,protons=0,neutrons=0))

after Matter.set_particle_collection(Particles(plist)) in physical_system.py. This way 'X' is always there as a vacuous ion, but it is not in the periodic_table which is queried for real data.

@shivupa
Copy link
Contributor Author

shivupa commented Jun 20, 2019

That makes more sense, and this way ghost atoms don't have to be named "X". I'll update this PR.

@shivupa shivupa changed the title Nexus: Adding support for ghost atoms [WIP] Nexus: Adding support for ghost atoms Jun 20, 2019
@jtkrogel jtkrogel self-assigned this Jun 21, 2019
@shivupa
Copy link
Contributor Author

shivupa commented Jun 21, 2019

@jtkrogel How does this look? I didn't love having to pop from kwargs since it is mutating the object, but it was necessary otherwise the __init__ for QmcpackAnalysisRequest would break.

@jtkrogel
Copy link
Contributor

Just one small requested change, then good to go.

@shivupa
Copy link
Contributor Author

shivupa commented Jun 21, 2019

Fixed!

@shivupa shivupa changed the title [WIP] Nexus: Adding support for ghost atoms Nexus: Adding support for ghost atoms Jun 21, 2019
@jtkrogel
Copy link
Contributor

Looks good. Thanks.

@prckent prckent merged commit a49a67e into QMCPACK:develop Jun 21, 2019
@shivupa shivupa deleted the nexus_ghost branch June 22, 2019 20:11
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.

5 participants