-
Notifications
You must be signed in to change notification settings - Fork 24
Added support for orca NEB. #833
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
99fddb8
55d80cb
ec88329
d6dc8f6
f51b1af
7c3d639
958510a
3724e96
7f1f52d
f00644b
1c26ddc
4543483
b83b405
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,379 @@ | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
| An adapter for executing Orca's Nudged Elastic Band (NEB) jobs. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Orca NEB: | ||||||||||||||||||||||||
| https://www.faccts.de/docs/orca/6.1/manual/contents/structurereactivity/neb.html | ||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import datetime | ||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||
| from typing import TYPE_CHECKING, List, Optional, Tuple, Union | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| from mako.template import Template | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| from arc.common import get_logger | ||||||||||||||||||||||||
| from arc.imports import incore_commands, settings | ||||||||||||||||||||||||
| from arc.job.adapters.common import is_restricted, which | ||||||||||||||||||||||||
| from arc.job.adapters.orca import _format_orca_method, _format_orca_basis | ||||||||||||||||||||||||
| from arc.job.factory import register_job_adapter | ||||||||||||||||||||||||
| from arc.job.local import execute_command | ||||||||||||||||||||||||
| from arc.level import Level | ||||||||||||||||||||||||
| from arc.parser.parser import parse_geometry | ||||||||||||||||||||||||
| from arc.species import TSGuess | ||||||||||||||||||||||||
| from arc.species.converter import xyz_to_xyz_file_format | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| from arc.species import ARCSpecies | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||||||||
| from arc.reaction import ARCReaction | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| logger = get_logger() | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| input_filenames, output_filenames, servers, submit_filenames, orca_neb_settings = \ | ||||||||||||||||||||||||
| settings['input_filenames'], settings['output_filenames'], settings['servers'], settings['submit_filenames'], \ | ||||||||||||||||||||||||
| settings['orca_neb_settings'] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| input_template = """ | ||||||||||||||||||||||||
| !${restricted}HF ${method} ${basis} NEB-TS | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| !${restricted}HF ${method} ${basis} NEB-TS | |
| !${restricted}${method_class} ${method} ${basis} NEB-TS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment might not be relevant, we're not looking st the species arg anymore here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why give the reactant xyz to initialize the TS object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can delete this comment (correct, but trivial - the err mssg is self-explanatory)
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The level retrieved from settings is a string (e.g., 'wb97xd/def2tzvp'), but it needs to be converted to a Level object before passing to the parent class. If level is None and orca_neb_settings.get('level', '') returns a string, this string should be wrapped in Level(repr=...) to ensure type consistency.
| level = level or orca_neb_settings.get('level', '') | |
| if not level: | |
| raise ValueError('A level of theory must be specified for Orca NEB jobs, either in the job arguments or in the settings file.') | |
| raw_level = level or orca_neb_settings.get('level', '') | |
| if not raw_level: | |
| raise ValueError('A level of theory must be specified for Orca NEB jobs, either in the job arguments or in the settings file.') | |
| # Ensure level is a Level object, even if provided as a string in settings | |
| if isinstance(raw_level, str): | |
| level = Level(repr=raw_level) | |
| else: | |
| level = raw_level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be done after super().__init__, so it's not overwritten?
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting self.execution_type at line 172 before calling super().init() is problematic because the parent class initialization will override this value based on the execution_type parameter passed at line 187. If you want to default to 'queue', you should either modify the execution_type variable before passing it to super (e.g., 'execution_type = execution_type or "queue"'), or set self.execution_type after the super call. The current approach may not have the intended effect.
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The abs_path is set to self.local_path, which will be incorrect for remote (queue) execution. When the job runs on a remote server, the local path from the client machine won't exist on the server. The NEB input file should use either relative paths (just 'reactant.xyz' and 'product.xyz') or self.remote_path for remote execution. Since files are uploaded to the execution directory, relative paths would be the safest and most portable solution.
| input_dict['abs_path'] = self.local_path | |
| input_dict['abs_path'] = '.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove 1 extra line break
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execute_incore method references incore_commands[self.job_adapter] where self.job_adapter is 'orca_neb', but there is no 'orca_neb' entry in the incore_commands dictionary defined in arc/settings/submit.py. This will cause a KeyError at runtime when attempting to execute orca_neb jobs incore. An entry for 'orca_neb' should be added to incore_commands, likely using the same command as regular orca jobs (e.g., 'orca input.in > input.log').
| execute_command([f'cd {self.local_path}'] + incore_commands[self.job_adapter], executable='/bin/bash') | |
| commands = incore_commands.get(self.job_adapter) | |
| if commands is None: | |
| # Fall back to the standard ORCA incore command if a specific orca_neb command is not defined | |
| commands = incore_commands.get('orca', []) | |
| execute_command([f'cd {self.local_path}'] + commands, executable='/bin/bash') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice implementation!