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

individually apply regex substitutions that add lines #1186

Merged
merged 3 commits into from Jun 5, 2017

Conversation

migueldiascosta
Copy link
Member

some of these (the ones with repeated matching patterns) were not being applied because of the way apply_regex_substitutions works

@migueldiascosta
Copy link
Member Author

@akesandgren please review? MPI is not getting enabled in 4.1-b2 because of this....

@akesandgren
Copy link
Contributor

Doh, didn't notice..

poatch looks good to me, but i haven't tested it.

@boegel
Copy link
Member

boegel commented Jun 1, 2017

@migueldiascosta Can we enhance the sanity check somehow to catch that MPI wasn't built? Checking for a particular file, or maybe running some command?

@boegel boegel added this to the 3.3.0 milestone Jun 1, 2017
@migueldiascosta
Copy link
Member Author

@boegel hm... echo "SystemName test" | mpirun -np 2 siesta 2>/dev/null | grep PARALLEL should return PARALLEL version...

@akesandgren
Copy link
Contributor

A ldd check of siesta should reveal a mpi library and avoid using mpirun.

ldd %(instalpath)s/bin/siesta | grep libmpi

should be non-null

@migueldiascosta
Copy link
Member Author

migueldiascosta commented Jun 1, 2017

@akesandgren that was actually not enough to detect, because it was linked with mpi, the problem was that -DMPI was not defined

@akesandgren
Copy link
Contributor

Ohhh, evil :-)

@migueldiascosta
Copy link
Member Author

yep :)

@boegel
Copy link
Member

boegel commented Jun 3, 2017

@migueldiascosta So, let's include that as a sanity check command then?

custom_commands = ["echo 'SystemName test' | mpirun -np 2 siesta 2>/dev/null | grep PARALLEL"]

super(EB_Siesta, self).sanity_check_step(custom_paths=custom_paths, custom_commands=custom_commands)

If grep doesn't find a matching line, it will exit with non-zero, and the sanity check will fail.

I tested this on top of your PR, works like a charm both to detect the broken installation and verify the fixed one, see migueldiascosta#1

add sanity check command in Siesta easyblock to verify parallel build
@boegel
Copy link
Member

boegel commented Jun 5, 2017

Thanks for the fix @migueldiascosta!

@boegel boegel merged commit 5c7197c into easybuilders:develop Jun 5, 2017
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.

None yet

3 participants