-
Notifications
You must be signed in to change notification settings - Fork 28
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
Hessian inversion #30
Comments
A very simple solution would be to ALWAYS set this egienvalue to the large positive value by default. We wouldn't change the "normal" operation where we try to get the natural TSFF, since the weight there is zero. Thus, to include it, we would only need to change the "eig_i" in constants.py |
Rather than writing separate arguments for calculate to handle Hessian inversion (past implementation), I think we should add an option to calculate that does the inversion to all other Hessian datatypes. Could work something like this.
This would read the Gaussian Hessian as per usual, but it would change element [0, 0] to be 1.5. Going further with this idea, the Hessian is read in many ways depending upon the filetype. However, it typically looks something like this, either stored directly on the file object or on a structure inside the file object.
There is already the function However, before I make that rediculously simple change, I am curious about peoples' opinions on this. Should |
Also, I just realized that my changes mentioned above would prevent anyone from doing inversion on some structures, while leaving the Hessian from some other structures as they are. Is this behavior okay with everyone, at least for now, for the sake of having this issue handled promptly (this enhancement could always be opened as a separate issue later if someone actually needs this feature)? |
And lastly (since I keep thinking of things), always inverting the Hessian as you mentioned Per-Ola would cause problems for optimizing GSFFs. This would certainly problematic for one of the projects currently going on in our lab that I think you have a particular vested interest in. |
OK, agree to the simple argument, and also, I didn't think of GSFF's, sorry... I cannot imagine a case where we only want to invert some Hessians. Only one thing: would you like to give the option of adding a second value after -i, for the weight? Every Hessian I have seen has been sorted so that [0,0] is also the smallest, so I have no strong preference, but [0,0] feels simple and robust. |
Okay, attempting to make this quick change in the next 10 mins. The additional weight option could probably be added if I thought about how argparse works. For the sake of time here, I am going to leave modifying the weight to changing the value |
If the optional argument --invert, -i is given, whatever floating point provided will be used to invert the Hessian whenever a Hessian is read from QM files. WARNING: Just noticed that the Gaussian hessian doesn't appear to be mass weighted (unless that happens elsewhere). Don't have time to look into this right now. OTHER WARNING: Also didn't have time to test ANY OF THIS CODE. Beware. Will update when I get more time. Helps with issue #30.
Now that I think about it, there are a few ways to do all of this. Could someone double check my thinking and also provide input as to what method would be ideal?
|
The Hessian can now be inverted using the command -i somefloat for calculate. This applies to -jh, -gh, -jeigz, -geigz. For -jh and -gh, the Hessian is decomposed into its eigenvectors and eigenvalues using numpy.linalg.eigh, the eigenvalue is inverted, and the Hessian is reformed. For -jeigz, we read the Hessian and eigenvectors. This is used to form the eigenvalue matrix. The diagonal is extracted (all off diagonal elements should be nearly zero), the lowest value is inverted, and then the eigenvalue matrix is reformed as a strictly diagonal matrix. For -geigz, we simply read the eigenvalues. The lowest value is inverted, and the eigenvalues are used to form a diagonal matrix. Addresses issue #30.
If the 1st method mentioned in my last post is okay with everyone, then I think this issue should be handled by commit f0b7c3c. |
A couple of things:
|
|
When you have the eigenvectors and eigenvalues, reforming the Hessian is trivial, you can go back and forth with H = V.W.VT W = VT.H.V As far as I see, we don't need those methods right now, I prefer what we currently use, multiply the eigenvectors onto the MM Hessian, and compare diagonal and off-diagonal elements separately. But don't throw the old methods away, we may want them in the future... (I've changed my mind before). |
Okay, so it sounds like we have to address bullet point 2 in my previous post before we can close this issue then. I will admit this isn't a high priority for me, since it's doing what we already have in different ways. |
Agree to low priority. How about temporarily deactivating the options, with a comment on the problem, but leave the functions in the code? |
I mean, is there a problem though? I don't see what we need to deactivate. Currently, we simply read the Hessian directly. I feel like that option should be left for the user. |
Fine for me, I'm not using those options currently. As long as we don't run the risk of a random user starting to compare apples with pears. |
I want to get point 2.) taken care of so we can close this issue. There are frequencies at the end of the file. They don't have many significant figures, but they're there. I already wrote code to read these, shown here. We could simply use those eigenvalues, but these are off from the current Can someone else double check my units here? Here's an example file. |
Don’t have much time now, but if you want the most number of decimals, pick the frequency, convert it to a.u. (factor in my HessMan.py program), square it, and put back the sign, should now be the eigenvalue of the mass-weighted Hessian. This works with both Gaussian and Jaguar. There is some kind of definition difference if you try to pick force constants and reduced masses from the two programs, they differ (and I don’t know the underlying reason), but the frequencies are the same for the same type of calculation. If you pick out the force constants, you have much fewer significant digits, and you have to do some kind of program-dependent conversion. /Per-Ola From: Eric Hansen [mailto:notifications@github.com] I want to get point 2.) taken care of so we can close this issue. There are frequencies at the end of the file. They don't have many significant figures, but they're there. I already wrote code to read these, shown herehttps://github.com/ericchansen/q2mm/blob/b5acb60dba4430cabadbd31b65f4489324d0a041/filetypes.py#L1016-L1018. We could simply use those eigenvalues, but these are off from the current -jeigz eigenvalues by about 4 decimal places. Can someone else double check my units here? Here's an example file. X001_E1.out.txthttps://github.com/ericchansen/q2mm/files/350378/X001_E1.out.txt — Confidentiality Notice: This message is private and may contain confidential and proprietary information. If you have received this message in error, please notify us and remove it from your system and note that you must not copy, distribute or take any action in reliance on it. Any unauthorized use or disclosure of the contents of this message is not permitted and may be unlawful. |
Revisiting what has to be done for this issue.
|
Where did the old technique with setting a large positive value for the negative eigenvalue go? We've mostly replaced it with the new, "natural" TSFF where we just set the weight to zero, but there are still some cases where we'd like the old technique available, and I can't find it now. Just to be clear, we do not need to create the modified Hessian, just replace the reference value for the first eigenvalue with a large positive in the reference data and use it with a non-zero weight.
The application I need it for right now is a final step where the force field has been converged to the "natural" values, taking out the force constants only for the breaking and forming bonds (and maybe some angles including them if they also ended up with too low values), and rerun ONLY those parameters with the modified Hessian. See Elaines and my 2015 article in JCC, the "F-SN2" TS, for motivation.
The text was updated successfully, but these errors were encountered: