Allowing get_aims_calculator to use K Grid Density about carmm HOT 7 CLOSED

My feeling is this should be trivial to get working in the first case, just by adding an if on this line: https://github.com/logsdail/carmm/blob/master/carmm/run/aims_calculator.py#L43 (and adding a default value on entrance to the function).

The other option is we use our own function to define the k-grid density, rather than the in-build FHI-aims functionality. We kind of did this previously, as one could define get_aims_calculator(..., get_k_grid(...), ...). You could in get_aims_calculator() just convert any given k-grid density into a k-grid, rather than passing on to FHI-aims? All are 2-3 line solutions.

from carmm.

The k_grid setting can have a type test, i. e. the tuple of integers would set the k_grid sampling same as now, and a float number (sampling density) would use get_k_grid() to obtain the (kx, ky, kz) tuple first.

How about setting up your branch of CARMM on Hawk and testing the proposed changes @OscarvanVuren? As @logsdail pointed out, this would be a good first commit.

from carmm.

The added if statements work correctly and put k_grid_density into calc.parameters. New problem is with ASE not recognising that a k-grid has been specified if using the new calculator.

EDIT:
Having looked in the ase.calculators.aims file on Hawk, the root of this issue becomes clear. In this script the code checks for a k-grid variable in the form of k_grid or kpts in the calc.parameters dictionary but it does not accept k_grid_density as a valid k-grid assignment. This is not the case in the source code for ASE from their website. I would guess that this is simply because it was not possible before to use a k-grid density with the code and ASE on Hawk is not up to date.

from carmm.

Yep, the change I made to the ASE code re: k_grid_density are now part of the master version. Perhaps test in local environment? Updates to the venv should be tested and discussed with the rest of the group first.

from carmm.

I’m happy to update the ASE version on Hawk if we know it won’t cause issues. What is the version on Hawk, and what do we need to raise it to?

from carmm.

I’ll have to put out a public service announcement with the update, but that’s all good.

from carmm.

The newest version of ASE doesn't seem to use calculators in the same way that it does now, instead applying profiles for each QM code to a general calculator template. As far as I can tell, making this work with the current version of Carmm would require rewriting a lot of the code.
I am going to close this issue for now as it seems to me that the solution isn't worth the time that would be spent on it.

from carmm.