Skip to content

Add NebCiWorkChain for climbing image NEB calculations#1133

Open
dbidoggia wants to merge 1 commit intoaiidateam:mainfrom
dbidoggia:ci-neb
Open

Add NebCiWorkChain for climbing image NEB calculations#1133
dbidoggia wants to merge 1 commit intoaiidateam:mainfrom
dbidoggia:ci-neb

Conversation

@dbidoggia
Copy link
Contributor

Dear all, I have created a NebCiWorkChain to first perform a no-CI NEB calculation followed by a refinement with climbing image using either automatic or manual method.

@mbercx
Copy link
Member

mbercx commented Sep 15, 2025

Thanks @dbidoggia! I think there may have been some git-shenanigans happening, because I see the commits from your previous PR, and we're also dealing with some conflicts. Is it ok if I adapt your PR to fix this?

@dbidoggia
Copy link
Contributor Author

Dear @mbercx, yes, absolutly. I tried everything, but could not understand why those commits still apear there

@mbercx
Copy link
Member

mbercx commented Sep 16, 2025

I think you must have branched off before pulling the latest changes in main. But thankfully a rebase on the aiidateam main branch seems to work like a charm:

git rebase origin/main
warning: skipped previously applied commit 7b505a2
warning: skipped previously applied commit 1b7a063
warning: skipped previously applied commit 3b6b7c4
hint: use --reapply-cherry-picks to include skipped commits
hint: Disable this message with "git config advice.skippedCherryPicks false"
Successfully rebased and updated refs/heads/pr/dbidoggia/1133.

Let me force push your ci-neb branch and see where we're at.

@mbercx
Copy link
Member

mbercx commented Sep 16, 2025

And all is well! (besides the RTD build, but I'll fix that later ^^) Thanks again for the contribution @dbidoggia. My main question before reviewing is: how stable is this workflow? As in: do you still expect to adapt it in the near future? I'm happy to add the workflow to aiida-quantumespresso, but that means once it's released we have to preserve backwards-compatibility, or deal with deprecations etc. Another approach we've been taking is to put more advanced workflows in separate packages (e.g. https://github.com/aiidateam/aiida-hubbard), which are still pre v1.0 and hence don't need to promise backwards compatibility between minor releases.

Something to consider, but again: also happy to review and add it to aiida-quantumespresso!

@dbidoggia
Copy link
Contributor Author

Thanks @mbercx, sooner or later I’ll learn how to avoid disasters on git…
This is a very basic WorkChain for now. Of course, it could be extended by adding checks on subworkchain outputs and refining the logic accordingly, but for our purposes, the current implementation is sufficient and I'm not planning to refine it deeper.

That said, I’ll leave the decision to you, since you probably know (or can better guess) the future perspectives coming from other contributions

@mbercx
Copy link
Member

mbercx commented Sep 18, 2025

Thanks for the comments @dbidoggia! I'm going to hold off on the review for a bit if that's alright, since I need to spend some time on understanding if it makes sense to add the workflow here. Moreover, I'm trying to switch to using hatch for our precommit etc. For this I want to focus on merging open PRs that could have horrible merge conflicts after updating the formatting, which this one shouldn't have an issue with.

Do let me know if you urgently need this merged for any reason. I'm happy to adapt priorities.

@dbidoggia
Copy link
Contributor Author

No, that's fine, don't worry

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 this pull request may close these issues.

2 participants