Skip to content

new psmpi easyblock, and environment variable setup in mpich.py flush…#852

Merged
boegel merged 8 commits intoeasybuilders:developfrom
damianam:psmpi_easyblock
Mar 11, 2016
Merged

new psmpi easyblock, and environment variable setup in mpich.py flush…#852
boegel merged 8 commits intoeasybuilders:developfrom
damianam:psmpi_easyblock

Conversation

@damianam
Copy link
Copy Markdown
Member

@damianam damianam commented Mar 2, 2016

New psmpi easyblock, and environment variable setup in mpich.py flushed out to its own method

@hpcugentbot
Copy link
Copy Markdown

Automatic reply from Jenkins: Can I test this?

@damianam
Copy link
Copy Markdown
Member Author

damianam commented Mar 2, 2016

@boegel , as promised. New easyconfigs PR coming

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 2, 2016

Jenkins: ok to test

@boegel boegel added this to the v2.7.0 milestone Mar 2, 2016
@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1739/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

Comment thread easybuild/easyblocks/p/psmpi.py Outdated
# and in a typical scenario we probably don't want them.
self.setup_env_vars()

# Bypass the configure_step of EB_MPICH
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mention why? (configure options defined there do not apply to psmpi?)

Comment thread easybuild/easyblocks/p/psmpi.py Outdated
if use_new_libnames:
libnames = ['mpi', 'mpicxx', 'mpifort']
else:
libnames = ['fmpich', 'mpichcxx', 'mpichf90', 'mpich', 'mpl', 'opa']
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how close is this to the MPICH sanity check?

looks like a lot of copy-pasting to me, the difference between psmpi and MPICH doesn't really stand out (and it should, imho)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how close is this to the MPICH sanity check?

looks like a lot of copy-pasting to me, the difference between psmpi and MPICH doesn't really stand out (and it should, imho)

@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1740/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@damianam
Copy link
Copy Markdown
Member Author

damianam commented Mar 2, 2016

@boegel all the suggested changes have been done. I haven't retested though.

Comment thread easybuild/easyblocks/p/psmpi.py Outdated
"""
Skip common options found in other MPICH-based runtimes
"""
pass
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it would be more elegant having an optional parameter in the configure_step in mpich.py. Something like:

def configure_step(self, set_default_options=True):

And then here, in psmpi.py something like:

def configure_step(self):
    [...] 
    super(EB_psmpi, self).configure_step(set_default_options=False)

@boegel would you agree?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like it better like this, since it's very explicit.

Maybe replace the pass with `self.log.info("Not using MPICH-specific configure options to configure psmpi")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just don't like the idea of doing and undoing (defining a method in mpich.py and overriding it to make it do nothing).

With an extra optional parameter in the configure_step method we avoid that, and it is also pretty explicit (you see it in the signature of the method).

Comment thread easybuild/easyblocks/p/psmpi.py Outdated

# configure, make and make install are default

def sanity_check_step(self, custom_paths=None, use_new_libnames=None):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can drop use_new_libnames here, since we don't have an easyblock that derives from EB_psmpi (and we'll probably won't have one soon)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for custom_paths btw, just drop it here (and below)

@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1744/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

Comment thread easybuild/easyblocks/p/psmpi.py Outdated
Disable the checking of the launchers for ParaStationMPI
"""
# cfr. http://git.mpich.org/mpich.git/blob_plain/v3.1.1:/CHANGES
# MPICH changed its library names sinceversion 3.1.1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix comment accordingly, any reference for psmpi?

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.

4 participants