-
Notifications
You must be signed in to change notification settings - Fork 307
Molpro: add the ability to specify a custom parallel launcher #928
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: develop
Are you sure you want to change the base?
Conversation
easybuild/easyblocks/m/molpro.py
Outdated
| } | ||
| run_cmd_qa(cmd, qa=qa, std_qa=stdqa, log_all=True, simple=True) | ||
|
|
||
| molpro_path = os.path.join(self.installdir, 'bin', 'molpro') |
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.
path is the same in both if and else bodies, so hoist it above the if?
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.
In fact, in one case self.installdir was used, and in the other, self.full_prefix, so not the same. However, I have set a variable destroot and used that instead, so that only one call to os.path.join is needed, and can be done outside the if statement.
|
@valtandor some minor remarks, please ping me when they're tackled since Travis doesn't notify of updates |
|
@boegel I will do, but it will need to wait for about two or three weeks as I'm on leave as of today with very limited access to my usual machines. |
|
@boegel: This should have the required changes implemented. |
easybuild/easyblocks/m/molpro.py
Outdated
| if self.cfg['parallel_launcher'] is not None: | ||
| apply_regex_substitutions(molpro_exe, [(launchertext, r'\1 "%s"' % self.cfg['parallel_launcher'])]) | ||
| elif self.orig_launcher is not None: | ||
| apply_regex_substitutions(molpro_exe, [(launchertext, r"\1 %s" % self.orig_launcher)]) |
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.
@valtandor while testing this, I noticed that the syntax here is not correct, i.e.:
$ grep '^LAUNCHER' $EASYBUILD_PREFIX/software/Molpro/*/*/bin/molpro
LAUNCHER= /path/to/software/impi/5.0.2.044-iccifort-2015.1.133-GCC-4.9.2/bin64/mpiexec -machinefile %h -n %n %x
the space after the =, that's just not right in a bash script...
This issue was already there in the easyblock however, you didn't introduce it. But we should fix this while we're at it.
This additional change fixes that problem, please review/merge: valtandor#4
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.
@valtandor ping?
make sure we don't insert a space after 'LAUNCHER=' in bin/molpro
This addition to the EasyBlock allows the user to specify, in the easyconfig, a launching string if a different MPI launcher is used (for example,
srun).