Feature/relax #451

moritzgubler · 2023-04-19T10:28:21Z

This is a draft of a geometry optimization implementation using the SQNM method. Variable cell shape optimization would also possible.

At the moment the geometry optimization input parameters are not parsed. Can someone help me with that?

I changed the main executable to always do a geometry optimization for testing purposes. To test the program, just compile it and run mrchem. A few parameters need to be set manualy:

world_prec = 1.0e-6
Properties {
  geometric_derivative = true          # Compute geometric derivative
}
SCF {
  path_orbitals = initial_guess              # Path to orbital files
  write_orbitals = true                # Save converged orbitals to file
  orbital_thrs = 1.0e-6
}

A tighter orbital_thrs reduces the noise in the forces significantly. The "path_orbitals" and "write_orbitals" need to be set like that because after the first geometry optimization iteration, I recycle the previous wave function as the initial guess for the next scf cycle to speed up convergence.

TODOLIST:

Parsing of input parameters
Final summary in the mrchem output file

stigrj · 2023-04-19T10:40:35Z

Thanks a lot @moritzgubler, this looks really interesting! I will have a closer look later this week.

moritzgubler · 2023-04-19T12:50:30Z

That sounds great @stigrj.

stigrj · 2023-04-22T13:56:20Z

@moritzgubler I made a pull request on your fork with an updated input parser.

src/vc_sqnm/mrchem_optimizer.hpp

moritzgubler · 2023-05-24T15:40:48Z

Hey @stigrj
I just acceptet your pull request and added some sentences to the documentation. There are some things that I can think of that we should do:

Think about the output of a geometry optimization. At the moment I just return a json that contains the molecule and the scf results of each iteration. Are you happy with that?
At the moment I write some useful info to cerr (this made it easier for me to separate it from the rest of the output). Do you use a logging library? Then we could redirect these prints to the logger.
It would be great if you could take a look at my energyAndForces function (line 56 in mrchem_optizer.hpp) to double check if I restart the calculation properly.
We could move the implementation of all functions into cpp files to speed up compilation.

Is there something I forgot?

codecov · 2023-05-24T16:34:35Z

Codecov Report

Attention: 284 lines in your changes are missing coverage. Please review.

Comparison is base (c1a5213) 71.64% compared to head (0cafe48) 70.31%.
Report is 1 commits behind head on master.

❗ Current head 0cafe48 differs from pull request most recent head 0c1ddde. Consider uploading reports for the commit 0c1ddde to get more accurate results

Files	Patch %	Lines
src/vc_sqnm/mrchem_optimizer.hpp	0.00%	133 Missing ⚠️
src/vc_sqnm/sqnm.hpp	0.00%	102 Missing ⚠️
src/vc_sqnm/periodic_optimizer.hpp	0.00%	27 Missing ⚠️
src/vc_sqnm/historylist.hpp	0.00%	20 Missing ⚠️
src/mrchem.cpp	88.23%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #451      +/-   ##
==========================================
- Coverage   71.64%   70.31%   -1.33%     
==========================================
  Files         186      190       +4     
  Lines       15022    15308     +286     
==========================================
+ Hits        10762    10764       +2     
- Misses       4260     4544     +284

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

stigrj · 2023-06-14T19:46:03Z

I will try to figure out a smart way to handle the output asap

moritzgubler · 2023-07-28T08:02:45Z

Do you have time to look at it soon?

ilfreddy · 2023-08-05T09:06:58Z

Vacation time right now in Norway, but I guess Stig will be able to have a look at it once he is back, or at least delegate it to some of us! 😃

src/vc_sqnm/README.md

src/vc_sqnm/historylist.hpp

src/vc_sqnm/mrchem_optimizer.hpp

robertodr · 2023-08-17T07:00:13Z

src/vc_sqnm/mrchem_optimizer.hpp

+
+    json results = energyAndForces(mol_inp, scf_inp, energy, forces);
+    // write progress to stderr for earier debugging.
+    std::cerr << i << " " << energy << " " << forces.cwiseAbs().maxCoeff() << "\n";


You should use the various printing facilities in MRChem to do this.

I don't really now what to print/ how to format the output. Do you have some printing guidelines?

These numbers are quite important to see if the geometry optimization converges. If they would just be printed, it is easy to overlook them because of all the other prints. Do you have an idea on how we could highlight this?

There is a Printer class that comes from MRCPP, which tries to handle formatting and such.¹ You can #include <MRCPP/Printer> and see here for some usage documentation https://mrcpp.readthedocs.io/en/latest/mrcpp_api/printer.html

I would say that this data should be part of both the JSON and the human-readable output files. When it comes to format for the latter, I don't have any great suggestions to offer, unfortunately. Maybe at the end of each optimization step a summary like printed in Psi4?

==> Convergence Check <== Measures of convergence in internal coordinates in au. Criteria marked as inactive (o), active & met (*), and active & unmet ( ). --------------------------------------------------------------------------------------------- ~ Step Total Energy Delta E MAX Force RMS Force MAX Disp RMS Disp ~ --------------------------------------------------------------------------------------------- ~ Convergence Criteria 1.00e-06 * 3.00e-04 * o 1.20e-03 * o ~ --------------------------------------------------------------------------------------------- ~ 1 -74.96466254 -7.50e+01 2.37e-02 1.83e-02 o 1.44e-01 8.71e-02 o ~ ---------------------------------------------------------------------------------------------

Footnotes

I'm not the expert on this, usually Stig Rune intervenes to cleansup the output... ↩

Hi @robertodr

Now, all the relevant prints are replaced by the mrchem printing api.

and I format an output json that gets written after the simulation. (see changes in mrchem.cpp)

src/vc_sqnm/periodic_optimizer.hpp

src/vc_sqnm/README.md

src/vc_sqnm/sqnm.hpp

moritzgubler · 2023-08-18T12:08:40Z

I implemented all your suggestions except for the printing and tested the code.

I have a question about the units. The parameters that will be set in my code are in Hartree/ Bohr and my code expects Hartree/ Bohr.

Some input parameters would change if the units were angstrom. For example the initial step size. Do you think it makes sense to change them?

robertodr · 2023-08-18T12:17:09Z

I implemented all your suggestions except for the printing and tested the code.

Thank you very much Moritz, especially for your patience! I can have a second review round in the weekend/Monday at the latest.

I have a question about the units. The parameters that will be set in my code are in Hartree/ Bohr and my code expects Hartree/ Bohr.

Some input parameters would change if the units were angstrom. For example the initial step size. Do you think it makes sense to change them?

I'm pretty sure that, at the level where you've interfaced your code, everything is guaranteed to be in atomic units already.

robertodr · 2023-08-18T12:17:46Z

I see that your branch is outdated with master and has accumulated some conflicts. Can you bring the branch up to date and fix the conflicts?

moritzgubler · 2023-08-18T12:24:04Z

The branch is up to date with master again.

src/vc_sqnm/mrchem_optimizer.hpp

moritzgubler · 2023-10-12T08:35:25Z

Hi @robertodr
I just brought the branch up to date with master again. Do you have time for another round of review in the next days?

src/vc_sqnm/mrchem_optimizer.hpp

not converge

stigrj · 2024-01-29T11:31:43Z

@moritzgubler sorry for my ridiculous response time on this. I actually thought there would be much more for me to consider here, but it turns out you have sorted out most things already 🙂 Great work!

I think your last suggestion with a separate JSON file for the geometry iterations is quite acceptable, so if you can just make that final change we can get this PR merged. Then the regular JSON output file should contain the original input but the output of the final geometry iteration. In this way the file should be backwards compatible.

I have resolved the conflicts in the input parser with a rebase and added some minor changes to the printed output. I pushed the fixes to the feature/relax branch on my fork : https://github.com/stigrj/mrchem/tree/feature/relax, so you can continue from there if you want.

moritzgubler · 2024-01-29T12:47:42Z

@stigrj No worries:)

I may have an even better idea: I adjust the mrchem output json the way you mentioned to make sure it is backwards compatible. The geometry optimization trajectory is written as a .xyz file. That way it is is easy to visualize the converged geometry. If you also agree with this, I will implement it like that.

stigrj · 2024-01-29T13:07:09Z

Is there any standardized format for such an xyz-file that other programs use? Will it contain only the molecular geometry? I think it will be good to keep a full record of all the properties (energy contributions, gradients, etc), as is currently done in the non-backward-compatible json we have now, only dump it into a separate file. But if there is a standard xyz-format that allows for immediate visualization of the trajectory, you can of course add that as well 🙂

moritzgubler · 2024-01-29T13:11:16Z

The .xyz format is pretty standardized and supported by standard visualization software. In that case, I will do both the json summary and the xyz output file.

src/vc_sqnm/mrchem_optimizer.hpp

moritzgubler · 2024-02-02T10:00:09Z

Hi @stigrj
I polished the xyz output, removed the debug prints, renamed the xyz file and the json summary file such that they share the base name of the original output.

moritzgubler · 2024-02-02T10:01:38Z

src/vc_sqnm/mrchem_optimizer.hpp

+    if (mrcpp::mpi::grand_master()) {
+        std::ofstream ofs;
+        ofs.open(jsonOutFileName + "_optimization_summary.json", std::ios::out);
+        ofs << summary.dump(2) << std::endl;
+        ofs.close();
+        xyzFile.close();
+    }


@stigrj is this the proper way to only write a file from the master process?

src/vc_sqnm/mrchem_optimizer.hpp

moritzgubler · 2024-02-05T13:23:39Z

@stigrj I think I adapted all the changes we recently talked about. I want to test one final thing regarding the xyz file output. I will contact you when I think the branch is ready to be merged.

moritzgubler · 2024-02-06T10:59:48Z

@stigrj The xyz writer is now also tested well and works. I have done and tested everything I wanted. If you also agree, we can merge it now.

stigrj · 2024-02-06T12:02:30Z

Great! Going in once the CI checks pass

moritzgubler commented Apr 25, 2023

View reviewed changes

src/vc_sqnm/mrchem_optimizer.hpp Show resolved Hide resolved

stigrj added the feature label May 23, 2023

moritzgubler marked this pull request as ready for review June 13, 2023 07:45