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

Add KspacingMetalHandler to VASP _DEFAULT_HANDLERS #600

Merged
merged 6 commits into from
Feb 15, 2024

Conversation

janosh
Copy link
Member

@janosh janosh commented Nov 3, 2023

In the context of materialsproject/custodian#298, @esoteric-ephemera and I noticed that ScanMetalHandler (now KspacingMetalHandler) is part of the default handler set in atomate1 but not used in atomate2 even though the new MP r2SCAN and all MatPES workflows use KSPACING.

This PR adds KspacingMetalHandler (requires new Custodian release once materialsproject/custodian#298 is merged) to the VASP _DEFAULT_HANDLERS. This should be safe to do since we fixed the handler in materialsproject/custodian#298 to not apply fixes on calculations that don't use KSPACING.

I also added a test for _DEFAULT_HANDLERS.

@utf
Copy link
Member

utf commented Jan 8, 2024

Hi @janosh, do you know why the tests are failing?

@janosh
Copy link
Member Author

janosh commented Jan 8, 2024

Needs a custodian release from @shyuep (please? 😅 )

@janosh janosh enabled auto-merge (squash) January 9, 2024 14:22
@janosh
Copy link
Member Author

janosh commented Jan 9, 2024

Thanks @shyuep for attempting custodian==2024.1.9. I fixed the release wf in materialsproject/custodian@16f58f0.

@utf
Copy link
Member

utf commented Feb 15, 2024

Hi @janosh, it seems like the custodian release isn't on Pypi? Are you able to push another one?

@utf utf mentioned this pull request Feb 15, 2024
13 tasks
@utf
Copy link
Member

utf commented Feb 15, 2024

Strange that the right version can't be found...

@janosh
Copy link
Member Author

janosh commented Feb 15, 2024

hmmm... pip install custodian==2024.2.15 works locally. tried both with pip 23.3 and 24 used in CI.

@janosh
Copy link
Member Author

janosh commented Feb 15, 2024

i would guess it's a cache issue. 80% sure this PR is safe to merge and will be green on main

@janosh janosh disabled auto-merge February 15, 2024 15:20
@janosh janosh merged commit e6ac30b into main Feb 15, 2024
3 of 6 checks passed
@janosh janosh deleted the add-KspacingMetalHandler-to-defaults branch February 15, 2024 15:20
@utf
Copy link
Member

utf commented Feb 15, 2024

Thanks for double checking!

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

Successfully merging this pull request may close these issues.

None yet

2 participants