MRChemSoft / mrchem

MultiResolution Chemistry
GNU Lesser General Public License v3.0
27 stars 21 forks source link

Remove runtest dependency #308

Closed robertodr closed 4 years ago

robertodr commented 4 years ago

MRChem can output its results in structured JSON format and is thus unnecessary to extract (via regex) results from the more user-friendly output files.

The new framework looks mostly like runtest: you still have to write a little test script in the folder with the input file. Notable differences:

  1. You have to check in under the reference folder the JSON output. You can check in the usual .out file too, but this one is not checked against anymore.
  2. The floating point comparisons are run à la NumPy. Given an absolute tolerance atol and relative tolerance rtol:
    abs(computed - expected) <= atol + rtol * abs(expected)
  3. The filters are given as a Python dict. The keys are tuples, the values a Tolerance object (a pair of absolute and relative tolerance values). The tuple is the address in the JSON file we need to probe for the value to check. For example, to check the computed sum of orbital energies to runtest's rel_tolerance=1.0e-6 you would do:
    filters = {("output", "properties", "orbital_energies", "sum_occupied"): Tolerance(rtol=1.0e-6, atol=1.0e-6)}

If you wanted to check to runtest's abs_tolerance=1.0e-6 you'd have to use Tolerance(rtol=0.0, atol=1.0e-6) You can find all the "addresses" in the JSON output in the schema: https://mrchem.readthedocs.io/en/latest/users/program_json.html#program-input-output-file

This is, of course, very tedious, hence I've put in some "aliases" for commonly used "addresses" and functions to mimic runtest tolerance set ups. For the above, you can do:

filters = {SUM_OCCUPIED: rel_tolerance(1.0e-6)}

and similarly for abs_tolerance.

With this change, MRChem is free from external Python packages for end-users: it can be downloaded, compiled, and run without having to create virtual environments.

codecov[bot] commented 4 years ago

Codecov Report

Merging #308 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #308   +/-   ##
=======================================
  Coverage   53.96%   53.96%           
=======================================
  Files         105      105           
  Lines        8403     8403           
=======================================
  Hits         4535     4535           
  Misses       3868     3868           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b3188f7...edf3530. Read the comment docs.

stigrj commented 4 years ago

Are any of the tests now run in MPI? Before we had mpirun -np 2 for h2o_energy_blyp, li_pol_lda and li_scf_pbe0, but I cannot see any mpirun in the new setup

robertodr commented 4 years ago

Are any of the tests now run in MPI? Before we had mpirun -np 2 for h2o_energy_blyp, li_pol_lda and li_scf_pbe0, but I cannot see any mpirun in the new setup

We still pass the launch agent option to the test script which then set it for the mrchem script: https://github.com/MRChemSoft/mrchem/pull/308/files#diff-7ecb0701ffeb2d223889e59042ff9a64R67-R68