Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation Inconsistency in ElasticMaker Input Set Modification Example. #1043

Closed
hongyi-zhao opened this issue Nov 5, 2024 · 5 comments · Fixed by #1047
Closed

Documentation Inconsistency in ElasticMaker Input Set Modification Example. #1043

hongyi-zhao opened this issue Nov 5, 2024 · 5 comments · Fixed by #1047

Comments

@hongyi-zhao
Copy link
Contributor

hongyi-zhao commented Nov 5, 2024

As an example, I've dug into ElasticMaker for several days to understand it deeply. Based on my current understanding, it seems that the code setting snippet given here is invalid:

# only update VASP jobs which have "deformation" in the job name.
new_flow = update_user_incar_settings(
    elastic_flow, {"ENCUT": 200}, name_filter="deformation"
)

In ElasticMaker, the two places for revising the input set are the bulk_relax_maker and elastic_relax_maker, by default, the former including two tight relax jobs, aka, tight relax 1 and tight relax 2, the latter will dynamically create N VASP static computation jobs named from elastic relax 1 to elastic relax N during the running process, where N <= 24, as noted here.

To summarize, some jobs in a flow are not real computations and there is no need to revise/update input set settings for them. If I'm wrong, feel free to correct me.

BTW, I go to ElasticMaker from the ElasticMaker listed here. From the results below, there are several ElasticMaker classes defined in atomate2, and in this case, it should be pointed to the first one shown below:

$ ug 'class[ ]+ElasticMaker' -g '*.py'
src/atomate2/vasp/flows/elastic.py:class ElasticMaker(BaseElasticMaker):
src/atomate2/forcefields/flows/elastic.py:class ElasticMaker(BaseElasticMaker):
src/atomate2/aims/flows/elastic.py:class ElasticMaker(BaseElasticMaker):

See Neraaz/HTESP#1 (comment) for the related discussion.

@JaGeo
Copy link
Member

JaGeo commented Nov 6, 2024

@hongyi-zhao I don't fully understand the issues.

Could you please produce a minimal example and tell us what is going different to what you are expecting. Thanks a lot.

@esoteric-ephemera
Copy link
Contributor

esoteric-ephemera commented Nov 8, 2024

@hongyi-zhao the documentation snippet for only updating jobs with deformation in the name just gives examples on how to update INCAR fields in jobs. It's not meant as practical advice.

You can see in the source code for update_user_incar_settings that a class filter is applied to restrict INCAR updates to VASP jobs. A utility / glue job that dynamically spawns VASP jobs will not be updated, but its dynamically-created VASP jobs could be

@JaGeo : the docs link to the wrong functions. The makers/flows in the VASP section of the docs link to FHI-AIMS. Can submit a fix quickly

@JaGeo
Copy link
Member

JaGeo commented Nov 8, 2024

PRs are welcome to the docs as well

@hongyi-zhao
Copy link
Contributor Author

hongyi-zhao commented Nov 9, 2024

@esoteric-ephemera

@hongyi-zhao the documentation snippet for only updating jobs with deformation in the name just gives examples on how to update INCAR fields in jobs. It's not meant as practical advice.

But it's very misleading, at least from my point of view when I try to learn it.

You can see in the source code for update_user_incar_settings that a class filter is applied to restrict INCAR updates to VASP jobs. A utility / glue job that dynamically spawns VASP jobs will not be updated, but its dynamically-created VASP jobs could be

Thank you very much for pointing this out.

@hongyi-zhao
Copy link
Contributor Author

hongyi-zhao commented Nov 9, 2024

@JaGeo

@hongyi-zhao I don't fully understand the issues.

Could you please produce a minimal example and tell us what is going different to what you are expecting. Thanks a lot.

It seems difficult for me to produce a minimal example, but @gpetretto and @esoteric-ephemera's above comment clarified the core idea that I want to express here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants