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

Fix initial cartesian units #3148

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

timostrunk
Copy link
Contributor

@timostrunk timostrunk commented Mar 21, 2024

Description

This PR fixes #3147 . Basically the psithon way of defining molecules converted _initial_cartesian on parsing.
When adding an external potential this was used as
molecule.set_geometry(clone_molecule._initial_cartesian).
set_geometry applies the conversion again.

The two python ways I found, which set initial_cartesian did not do the conversion leading to incorrect geometries, when using external_potential and molecules defined as unit=angstrom.

User API & Changelog headlines

Dev notes & details

Questions

  • Question1

Checklist

Status

  • Ready for review
  • Ready for merge

@timostrunk
Copy link
Contributor Author

I ran pytest quick tests, all pytest still running

@timostrunk
Copy link
Contributor Author

All tests ran through except for:
FAILED tests/fsapt1/test_input.py::test_fsapt1 - AssertionError: Traceback (most recent call last):
with the message:
E !----------------------------------------------------------------------------------!
E ! !
E ! Fatal Error: SCF_SUBTYPE=INCORE was specified, but there is not enough memory to !
E ! do in-core! Increase the amount of memory allocated to Psi4 or allow for !
E ! out-of-core to be used. !
E ! Error occurred in file: /home/conda/feedstock_root/build_artifacts/psi4nm_171103 !
E ! 8779956/work/psi4/src/psi4/lib3index/dfhelper.cc on line: 261 !
E ! The most recent 5 function calls were: !
E ! psi::PsiException::PsiException(std::__cxx11::basic_string<char, !
E ! std::char_traits, std::allocator >, char const*, int) !
E ! psi::DFHelper::initialize() !
E ! !
E !----------------------------------------------------------------------------------!

I think this is independent of this PR and the PR would be good to go

Copy link
Member

@jturney jturney left a comment

Choose a reason for hiding this comment

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

Seems pretty straightforward. Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using external potential in python broken in case of Angstrom units
3 participants