Fixes reference INCARs for MPSCANRelaxSet calculations to correspond …

[MichaelWolloch]

…to the current setting of LELF.

Summary

Updated all reference INCARs for calculations done with the MPSCANRelaxSet to have

LELF = False

according to materialsproject/pymatgen@1fa96f8

Please also see discussion at the end of #788

Note that two tests will still fail, with the first one being addressed in materialsproject/pymatgen#3204 :

• atomate/vasp/workflows/tests/test_nmr.py::TestNMRWorkflow::test_wf
• atomate/qchem/firetasks/tests/test_fragmenter.py::TestFragmentMolecule::test_babel_PC_with_RO_depth_0_vs_depth_10

[MichaelWolloch]

@Zhuoying , this should be a quick merge. Nothing fancy.
However, I am not sure how to fix the issue with the missing requirements-ci.txt for the CI file that I removed in my last PR. I updated .github/workflows/test.yml, which makes sense, but it is still trying to read the file. I am not familiar with CircleCI unfortunately, but I am happy to take instructions.

[Zhuoying]

Hi @MichaelWolloch, thanks for the INCAR fixing. I saw your removal of requiements-ci.txt in the last PR and modification in .github/workflows/test.yml, but there is another line mentioning requiements-ci.txt here.

atomate/.github/workflows/test.yml

Line 29 in 52608a1

requirements-ci.txt

I think keeping the CircleCI tests is necessary because:

1. it provides a flexible configuration setting for testing pipelines
2. it can be triggered for a specific branch rather than main only.

To resolve this, @MichaelWolloch could you add requiements-ci.txt and requiements.txt back? In .github/workflows/test.yml, to install all necessary dependencies (including requiements.txt), we might need to add this line:
https://github.com/materialsvirtuallab/pymatgen-analysis-diffusion/blob/9e8773f9784772489275738e26ae3a00bae7f16b/.github/workflows/testing.yml#L30

@janosh any thoughts on this?

[janosh]

To resolve this, @MichaelWolloch could you add requiements-ci.txt and requiements.txt back? In .github/workflows/test.yml, to install all necessary dependencies (including requiements.txt), we might need to add this line:

I vote against adding req files back. Better to work with setup.py install_requires and extras_requires imo.

I think keeping the CircleCI tests is necessary because:

1. it provides a flexible configuration setting for testing pipelines
2. it can be triggered for a specific branch rather than main only.

CircleCI has beefier machines so the tests run faster than in GH Actions which is always nice. So no harm in keeping them. But GH Action offers the same feature of picking which branch to run on:

All you need is a workflow_dispatch: trigger in the action's config.

[Zhuoying]

CircleCI has beefier machines so the tests run faster than in GH Actions which is always nice. So no harm in keeping them. But GH Action offers the same feature of picking which branch to run on:

@janosh Good to know GH action works for any selected branch.
Is there any reason that other MP repos still keep at least requirements.txt as far as you know? -- I just checked Pymatgen, Custodian, and Fireworks, they are still updating requirements.txt even if they deprecate CircleCI for testing. Thus I kind of think, whether to keep req files and whether to keep CircleCI are two things.

[janosh]

Thus I kind of think, whether to keep req files and whether to keep CircleCI are two things.

That's right, totally independent.

Is there any reason that other MP repos still keep at least requirements.txt as far as you know?

No good reason. The source of truth for package deps is setup.py or more recently pyproject.toml. requirements.txt files are just noise imo.

[MichaelWolloch]

Hello @Zhuoying and @janosh, thanks for the clarification!

Hi @MichaelWolloch, thanks for the INCAR fixing. I saw your removal of requiements-ci.txt in the last PR and modification in .github/workflows/test.yml, but there is another line mentioning requiements-ci.txt here.

atomate/.github/workflows/test.yml

Line 29 in 52608a1

requirements-ci.txt

This is exactly what I removed in 7adc24d, but I did not look for any other mentions of the file, missing .circleci/config.yml.

I did not get at all that there are two different ways to run the tests.

Thus I kind of think, whether to keep req files and whether to keep CircleCI are two things.

That's right, totally independent.

Is there any reason that other MP repos still keep at least requirements.txt as far as you know?

No good reason. The source of truth for package deps is setup.py or more recently pyproject.toml. requirements.txt files are just noise imo.

I did update .circleci/config.yml in the end instead of putting requirements*.txt back. I agree with @janosh that it is not really beneficial to have two versions of the same info out there.

[janosh]

Thanks @MichaelWolloch! 👍