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

psi: initial Psi4 Mol extensions #44

Merged
merged 2 commits into from
Oct 19, 2018
Merged

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Apr 16, 2018

Description

Molecule JSON exported from Psi4 has several extra fields already checked for self-consistency so that the Mol may henceforth be used without reconsulting a periodic table. This defines the fields of that main psi4mol extension. There are also two more extensions, one for EFP fragments and one for transmitting ZMat mol specifications.

Questions

  • I tried to follow guidelines here. Properly there should have been a secondary repo. Here I just made an extensions folder. I'm sure that won't do for structuring a complex schema. I'll investigate further.
  • The minimal required input fields for EFP are actually fragment_files, hint_types, geom_hints, and units. All other fields are valid after and EFP system has been populated by an EFP-reading program into a consistent, along-side "viz" representation. These alternate sets of "required" fields have not yet been programmatically imposed.

Status

  • Ready to go

@dgasmith
Copy link
Collaborator

This is interesting as it is a formalization of the considered extensions. I would be interested in @ghutchis's (and others) thoughts on this.

@loriab
Copy link
Collaborator Author

loriab commented Oct 17, 2018

This original molecule extension PR will be split.

  • what remains in the PR is the bit that should be incorporated into moleucle_schema proper, which is 4 new optional keys
  • from offline discussions it sounds like 9cf3dff#diff-fb40813be873a1ea003de289cb1f9373R6 and following line should be deleted. and the whole schema bumped from 1-->2. My opinion is that, while we shouldn't go overboard on segmenting the schema, having a few separate pieces doesn't hurt, especially when there are whole software projects that only deal with one piece (e.g., molviz and basissetexch)
  • remaining pieces of original PR now missing in this PR are zmat and efp extensions. Are separate or combined extension PR(s) wanted for that?

Copy link
Collaborator

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

I think we should get this in and not yet cut a new release until we have the multiple schema for molecule/input/output worked out.

@dgasmith
Copy link
Collaborator

Can you make an issue about z-mat and EFP and discuss the extensions there? I don't think we have a good framework of where to put these yet.

@dgasmith dgasmith merged commit cc151bc into MolSSI:master Oct 19, 2018
@loriab loriab deleted the psimolext branch October 23, 2018 18:21
loriab added a commit to MolSSI/QCElemental that referenced this pull request Oct 23, 2018
loriab added a commit to MolSSI/QCElemental that referenced this pull request Oct 23, 2018
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

2 participants