[FIX] fix elastic tensor flow

[mjwen]

Summary

Closes #333.

The elastic tensor flow currently cannot generate the appropriate number of (24) deformation tasks to fit the tensor if sym_reduce is used. The primary reason is using the deformation gradient to check independent calculations as detailed nicely in #333. This PR fixes it by using strain to check for independence.

In a comment on #333, I mentioned that in emmet, we have a more complex way to deal with it. That is not needed here. This is because, in atomate2, the deformation tasks are generated based on the relaxed structures, while in atomate1, it is based on the initial structure. The atomate1 approach can have an issue when the symmetry changes after relaxation, so we add additional guards there. In atomate2, there is no such issue.

Fix conventional cell bug

If conventional is set to True, the flow only uses a conventional structure for generating the deformations. But the structure used for later elastic calculation and tensor fit is still the original structure. As a result, the symmops used to reduce and expand the explicit calculations can then be different, because one is determined based on the conventional structure, and the other based on the original structure. Also, one should not use deformations from a conventional cell to strain the original structure.

This PR fixes this by converting the structure to a conventional structure and using it in all calculations. This is also how atomate1 does it.

Changes

• Replace deformation with strain to expand the set of calculations (this fixes the bug)
• Replace deformation with strain to reduce the set of structures for explicit calculation. This can have two major benefits:
  • possibly further reduce the number of deformed structures for calculation. This is because the strain is a symmetric tensor while the deformation gradient is not.
  • consistent with the new _expand_strains() function
• Correct reference values for elastic flow test
• Add comprehensive tests for using reduced structures, conventional cells, and reducing/expanding strains

[mjwen]

Just added tests for reducing the deformed structures, and for recovering from them. The idea is to test the set of strains before reducing and after recovering are exactly the same.

The test is set up, but it is only testing for a single structure now. I hope to test for all 230 space groups. There are two options to move forward:

1. create structures on the fly
2. add a test file that consists of all needed structures

I prefer the first approach, but it seems not easy to create a structure from each space group using e.g., Structure.from_space_group(), which requires many args. For the second approach, we can simply query from MP a prototype structure for each space.

Any suggestions @utf and @mkhorton?

[utf]

The test is set up, but it is only testing for a single structure now. I hope to test for all 230 space groups. There are two options to move forward:

I don't actually think there are examples of all 230 space groups in the materials project. It should be sufficient to pick one example from each of the 14 Bravais lattice. I already compiled a list of example structures in amset. See here: https://github.com/hackingmaterials/amset/tree/main/tests/test_data/structures

[mjwen]

I've also fixed a conventional cell bug; see the Summary above.

The tests are failing because the "TCH", # flake8-type-checking stuff will autofix and add TYPE_CHECKING for ElasticDocument and put Structure and other stuff inside its if block. This does not work with pydantic.

I was trying to fix it by adding something below to pyproject.toml, but did not get luck. Before digging deeper, I think @janosh should be able to help with this.

[tool.ruff.per-file-ignores]
"common/schemas/*" = ["TCH"]

After that is fixed, it is ready for review.

[janosh]

I think the best thing to add would be

[tool.ruff.per-file-ignores]
# things inside TYPE_CHECKING aren't available at runtime and so can't be used by pydantic models
"**/schemas/*" = ["TCH"]

to disable moving imports into TYPE_CHECKING in all pydantic schema files. That should permanently solve this problem. It didn't work for you because paths are relative to repo root, so you need to add leading **.

[mjwen]

@janosh I know you should fix it quickly. Thanks!

Another fix you might be able to help: Ruff does the below stuff to use newer typing. it does not work with older versions of Python (causing the below testing error). Any idea to disable it for schema files?

[janosh]

@mjwen I think we want to keep those ruff changes. from __future__ import annotations is a side effect import that makes builtins like list, dict, set, etc. work as type hints from Python v3.7 upwards. Given the lowest Python supported by atomate2 is 3.8, it should be safe to use. What version do you have?

[mjwen]

Yes, it would be great to keep these. I am using py3.10 on my laptop and everything works nicely.

It seems py3.8 does not work, which results in the error you see in the GH action below: "testing / test (3.8) (pull_request)".

So, I guess we will need to either upgrade the lowest supported version to py3.9 (introduced in https://peps.python.org/pep-0585/ for py3.9)?, or use List instead of list.

Pydantic is a bit different. The from __future__ import annotations makes list and the like work for typing hint for older versions of python, but not for Pydantic.

[janosh]

@mjwen Sorry, didn't notice that we're still talking about a schema file. In that case the situation is different. pydantic models don't support future annotations until 3.10. So seems best if we also ignore FA in all schema files

# flake8-type-checking (TCH): things inside TYPE_CHECKING aren't available
#     at runtime and so can't be used by pydantic models
# flake8-future-annotations (FA): future annotations only work in pydantic models in python 3.10+
"**/schemas/*" = ["FA", "TCH"]

[mjwen]

In that the situation is different. pydantic models don't support future annotations until 3.10.

Good to know!

And I agree that yours is the best solution.

[codecov]

Codecov Report

Merging #415 (f03fdf3) into main (95b15d5) will decrease coverage by 0.14%.
The diff coverage is 82.60%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #415      +/-   ##
==========================================
- Coverage   65.30%   65.16%   -0.14%     
==========================================
  Files          74       74              
  Lines        7186     7186              
  Branches      927      929       +2     
==========================================
- Hits         4693     4683      -10     
- Misses       2208     2220      +12     
+ Partials      285      283       -2     

Files Changed	Coverage Δ
src/atomate2/utils/file_client.py	43.82% <0.00%> (ø)
src/atomate2/vasp/flows/elastic.py	77.27% <25.00%> (-5.23%)	⬇️
src/atomate2/common/schemas/elastic.py	88.77% <100.00%> (+0.11%)	⬆️
src/atomate2/vasp/jobs/elastic.py	82.05% <100.00%> (+3.73%)	⬆️

... and 1 file with indirect coverage changes

[mjwen]

Thanks @janosh! Now all typing and such checks are done.

@utf ready for review.

[utf]

Fantastic, thank you.