search

eljost / pysisyphus

Python suite for optimization of stationary points on ground- and excited states PES and determination of reaction paths.
GNU General Public License v3.0
89 stars 31 forks source link

Enhancements to Psi4 Handling, Thread Management, and Alignment for Calculations #258

Closed Marclie closed 1 year ago

Marclie commented 1 year ago

This pull request includes three significant improvements for the pysisyphus package:

  1. The no_reorient and no_com options have been added to the generated input files from the Psi4 calculator, preventing undesired modification of geometry data.
  2. The thread distribution strategy has been revised when using the Dask scheduler with the pal flag, improving performance and efficiency for concurrent calculations.
  3. A new flag, align_factor, has been implemented to allow partial alignment in calculations that have an energy dependence on the molecular orientation, addressing a key issue in optimization convergence for such cases.

Please refer to the individual commit messages for more detailed explanations of each change. Thank you for considering these enhancements.

Marclie commented 1 year ago

Hello Johannes,

Thank you for your review and constructive feedback.

The alignment factor is definitely a niche use-case specific to my research, where I am generating reaction pathways of light-catalyzed reactions using quantum electrodynamics. In these scenarios, light is bound to the molecular dipole moment and the dipole's alignment with the light's polarization affects the total energy. Forced full alignment causes instability when the gradients should rotate the molecule. However, I still needed some alignment between images. The align_factor solved the problem for me after some fine-tuning.

As I am still figuring out the details of Pysisyphus, I chose to use a static variable for align_factor to avoid possibly breaking any functionalities. I appreciate your insight to know that only a few classes needed small changes. Similarly, I wasn't certain whether pal is consistent for all images in the COS calculations (the ONIOM model gave me a few doubts, as mentioned in my commit). It is much cleaner treating each image equally when distributing threads for Dask.

I hope these changes are useful and your time and attention is much appreciated.

Best Regards, Marcus

eljost commented 1 year ago

Dear Marcus, i sent you some additional commits in a PR (https://github.com/Marclie/pysisyphus/pull/1). When you merge them, I can merge this PR into pysisyphus.

Marclie commented 1 year ago

Hello Johannes, I have merged your fixes to my PR. As I gave in a comment, there is just one extra small change that I made that I think needs to be included in this PR. The optimizer classes now do a great job at handling my orientation-dependent calculations. However, I discovered that the fixed images in an NEB could still be reoriented in the set_coords_at function from ChainofStates.py when aligning along the path. I've been using free-end neb and did not test until now a reaction path with a sharp rotation, so this effect went unnoticed. The small change in the commit before I merged addresses this.

eljost commented 1 year ago

Thanks for your contributions!