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

basisset renormalization #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

basisset renormalization #230

wants to merge 1 commit into from

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Jul 14, 2020

Description

The originating difficulty is that there are two uses for BasisSet and ElectronShell schema, and they've chosen different coefficient normalization schemes for good reason.

  • BSE uses unnormalized, the way the literature publishes them. These don't have a fixed scaling, so can't recreate them by formula. Most QC programs are only equipped to slurp up basis set specs with these coefficients.
  • AtomicResult wavefunction is based in CCA and so the BasisSet that accompanies orbitals is based in CCA normalization. Requiring extra logic to adapt BSE coefs to relate BasisSet to Wavefunction is what want to avoid in qcsk.

My problem that prompted this is I want to create a psi4.core.BasisSet from a qcsk BasisSet (in particular, after a finite different gradient with distributed driver). Presently, psi takes in bse unnorm, then computes erd and cca, then stores all three. It really is most flexible to deal with bse unnormalization.

This PR adds a member data normalization_scheme that is autodetected upon initialization (falls back to bse). It can convert in any direction among original/unnorm/BSE, CCA, and ERD coefficients, as implemented in psi, via new_bs = bs.normalize_shell(dtype="cca"). My thought is that qcsk exports of wfn for AtomicResults can specify a normalization scheme the orbitals expect the basisset to exhibit, then even if the basisset is exported in a different scheme, easy to convert on the fly. More normalizations can be added, of course.

This is the full scope I plan for qcel, as I'm next moving on to psi4 parts. @dgasmith and @bennybp should have the opportunity to raise concerns and views.

Questions

  • does ElectronShell now need a schema version number? or should BasisSet be incremented?
  • NormalizationScheme.bse means within a uniform scale factor of published coefs. I haven't yet needed to differentiate "scaled_bse" and "official_bse". Any reason to add that complication now?

Changelog description

Status

  • Code base linted
  • Ready to go

@codecov
Copy link

codecov bot commented Jul 14, 2020

Codecov Report

Merging #230 into master will increase coverage by 0.08%.
The diff coverage is 96.94%.

@dgasmith
Copy link
Collaborator

Cool, no objections here.

@wilhadams
Copy link

This is extremely relevant to the QCSchema issue MolSSI/QCSchema#12. It can be hard to specify a convention scheme without explicitly documenting it, as @tovrstra mentioned, so it would likely make more sense for the QCSchema to support a uniform notation for specifying a convention, instead of limiting the possible normalizations to specific conventions (which would require programs implementing QCSchema that are not using those conventions to add renormalization functionality anyway).

Either way, for the QCSchema as a schema, the decision should be between requiring a specific normalization or requiring an additional keyword to specify the normalization. Obviously both options have consequences in terms of implementation, but there will be overhead for implementing QCSchema in any case.

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.

3 participants