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

add hashes and rearrange mult handling #343

Merged
merged 4 commits into from
Sep 18, 2024
Merged

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Jul 26, 2024

Description

This PR is preparatory to #318.

  • NEW: adds a check to the basic Molecule pydantic model so multiplicities <1 get caught. This route is accessed when Molecule(..., validate=False). I believe this is a universally good check, but it is a new restriction. Any weird positron, fractional electron, etc. reason to not catch less-than-one mult?
  • add many hash checks (incl. all the chgmult variations in validate_and_fill_chgmult testing) to molecules, so we'll know if code starts changing the hash and thereby disrupting the QCA server.
  • qcel.molparse has a different error message for less-than-one fragment_multiplicities you try to put through it. qcel.molparse is accessed whenever Molecule(...) or Molecule(..., validate=True) is called.
  • qcel.molparse newly allows floats that are ints (e.g., 1.0) for multiplicity. previously it would raise an error about not being an int.
  • qcel.molparse newly has a dedicated error for any less-than-one molecular_multiplicity you try to put through it. Previously it raised the usual "Inconsistent" error.
  • more tests that chgmult are processing as expected, including some with fractional charges (https://github.com/MolSSI/QCElemental/pull/343/files#diff-29ef453faf072f4ee853122f0f64f6ba594c00dc44599a1f5138946619d9951fR90-R96) for the first time.

Changelog description

Status

  • Code base linted
  • Ready to go

qcelemental/tests/test_molecule.py Fixed Show fixed Hide fixed
qcelemental/tests/test_molecule.py Fixed Show fixed Hide fixed
@loriab
Copy link
Collaborator Author

loriab commented Jul 29, 2024

@awvwgk , is enforcing a lower bound of 1.0 on multiplicity a sensible thing to do even for fractional electron, or is it restricting?

@awvwgk
Copy link
Contributor

awvwgk commented Jul 29, 2024

I believe a lower bound of 1.0 is sensible, I can't think of a usecase even for a fractional electron which could not be represented this way.

Derk @DerkKooi, do you know any case that is not representable with this lower bound of multiplicity?

@loriab loriab merged commit 936407e into MolSSI:master Sep 18, 2024
17 checks passed
@loriab loriab deleted the prefloatmult branch September 18, 2024 03:37
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