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

Check all frequencies in detrotra #1205

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

Conversation

foxtran
Copy link
Contributor

@foxtran foxtran commented Feb 27, 2025

Fixes #1203

H2 eigvals are the following:

2.9933945092181844E-015   8.5529038621230392E-015   2.6862916288779701E-014   3.4268547221618851        3.4268547221618966        40.610372376753845

@foxtran
Copy link
Contributor Author

foxtran commented Mar 5, 2025

@marvinfriede, can we merge it?

@Albkat Albkat requested a review from marvinfriede March 12, 2025 14:30
@foxtran foxtran force-pushed the fix/1203 branch 6 times, most recently from c51703f to a4d8cfe Compare March 16, 2025 19:12
@foxtran
Copy link
Contributor Author

foxtran commented Mar 16, 2025

@marvinfriede, ping :-)

@thfroitzheim
Copy link
Contributor

Isn't this the same issue we see in #1210. For linear water, it appears to be around 900 wavenumbers if I see it correctly. Why do we limit this at all? Shouldn't the ranked value be correct even if we check for all modes? If not, we need to introduce a check that at least 5 or 6 values are included for linear and non-linear molecules. Otherwise, it will fail again, right?

@foxtran
Copy link
Contributor Author

foxtran commented Mar 25, 2025

Why do we limit this at all?

Now, there is no limits. All freqs will be checked.

Isn't this the same issue we see in #1210.

They are different.

@foxtran foxtran changed the title Increase threshold for lowest frequencies Check all frequencies in detrotra Mar 25, 2025
@foxtran
Copy link
Contributor Author

foxtran commented Mar 27, 2025

Ah.. It always checks 5/6 freq., but only 3 can be imaginary :(

@thfroitzheim
Copy link
Contributor

Are we actually sure that this is the right way of finding rotations and translations? As far as I know, one normally projects rotations and translations out so that the final hessian has 5/6 degrees of freedom less. The remaining degrees of freedom are then all vibrations. We have code to do something like this here, but we still try to assign frequencies to the full hessian. Maybe it is better to do it properly via a projection to make sure it works in all cases.

@foxtran
Copy link
Contributor Author

foxtran commented Mar 29, 2025

It seems to me that this part was never been adjusted for such cases. So, let it be as is without significant updating like in #1210

Igor S. Gerasimov and others added 2 commits March 29, 2025 16:13
Signed-off-by: Igor S. Gerasimov <[email protected]>
Signed-off-by: Igor S. Gerasimov <[email protected]>
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.

Segmentation fault for scan / constrained optimization for diatomic molecules
2 participants