Skip to content

Remove deprecated configure options from MVAPICH2 easyblock#853

Merged
boegel merged 14 commits intoeasybuilders:developfrom
besserox:cleanup_mvapich2
Mar 11, 2016
Merged

Remove deprecated configure options from MVAPICH2 easyblock#853
boegel merged 14 commits intoeasybuilders:developfrom
besserox:cleanup_mvapich2

Conversation

@besserox
Copy link
Copy Markdown
Contributor

@besserox besserox commented Mar 3, 2016

MVAPICH2 easyblock uses some deprecated configure options:

  • --enable-mpe disappeared starting version >= 1.9
  • --enable-hwloc disappeared starting version >= 2.0

This PR removes these deprecated options from the easyblock.
PR easybuilders/easybuild-easyconfigs#2626 for easyconfigs set these options directly in the easyconfig files when there are used.

PR easybuilders/easybuild-easyconfigs#2626 have to be merged first.

@hpcugentbot
Copy link
Copy Markdown

Automatic reply from Jenkins: Can I test this?

extra_vars = {
'withchkpt': [False, "Enable checkpointing support (required BLCR)", CUSTOM],
'withmpe': [False, "Build MPE routines", CUSTOM],
'withhwloc': [False, "Enable support for using hwloc support for process binding", CUSTOM],
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.

these shouldn't be removed

although we can update the easyconfigs we ship ourselves accordingly, we should forget that there are other easyconfigs out there we do not control...

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 3, 2016

Jenkins: ok to test

@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1748/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.

@boegel boegel added this to the v2.7.0 milestone Mar 3, 2016
@besserox
Copy link
Copy Markdown
Contributor Author

besserox commented Mar 4, 2016

Just some notes, for later:

  • About MPE
    • --enable-mpe disappeared starting MVAPICH2 version >= 1.9
    • It is actually an option coming from MPICH, not MVAPICH2.
  • About HWLOC
    • --enable-hwloc disappeared from configure starting MVAPICH2 version >= 2.0
    • HWLOC is apparently distributed with MVAPICH2 and always compiled
    • --enable-hwloc/--disable-hwloc still appears in the user guide (but I guess they forgot to update this)

@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1761/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.

@besserox
Copy link
Copy Markdown
Contributor Author

besserox commented Mar 7, 2016

@boegel This PR is not doing much now except documenting a bit more the issues.

  • It raises an error if 'withmpe' is used for MVAPICH2 1.9 or above
  • If 'withhwloc' is false or not specifed (false is the default value), MVAPICH2 2.0 or above will still use HWLOC anyway because it not optional anymore.

Let me know if you have a better behavior to propose.

Comment thread easybuild/easyblocks/m/mvapich2.py Outdated
# and it cannot be disabled.
# The 'withhwloc' option should be maintained for backward compatibility purpose.
# EasyBuild and MVAPCH2 will just silently ignore this option if it is used.
add_configopts.append('--with-hwloc')
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.

is this actually still a known configure option in MVAPICH >= 2.0?

if not, we should not use it anymore...

cfr. #157

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Two things:

  • --with-hwloc is not recognized by the configure (but it is still in the documentation). I can clarify this with the MVAPICH2 team.
  • The configure from MPICH/MVAPCH2 is not checking for the unrecognized options. Because they use a top-level configure which is calling many other configure scripts with all the same options (some options being used by some configure scripts, and not by other ones). So trip over unknown configure options #157 will not apply here.

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.

OK, but does it make any sense to include it, since it has no effect?

So, we should just skip it and issue a warning for MVAPICH >= 2.0?

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 7, 2016

@besserox: the logic makes sense to me, but we could do a little bit better (cfr. remarks)

besserox added 5 commits March 9, 2016 18:17
This does not change the behavior of old easyconfig files because:
- previously, if withhwloc was False, we did not set explicitly '--without-hwloc'
- and if '--without-hwloc' was not set, the actual default behavior of the configure was to enable it
@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1811/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.

@besserox
Copy link
Copy Markdown
Contributor Author

@boegel This should be better now.

One change is that the default value for withhwloc is now True. But this does not change the behavior of old easyconfig files because:
- previously, if withhwloc was False, we did not set explicitly --without-hwloc
- and if --without-hwloc was not set, the actual default behavior of the configure was to enable it

So now I'm able to find out detect if the easyconfig set withhwloc to False for MVAPCH2 >= 2.0 and raise an error.

spacing cleanup, easier to follow logic, fix long line
@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1825/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.

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 11, 2016

@besserox: this needs besserox#6 to fix a merge conflict (because #854 was merged)

sync with develop & resolve conflict
@hpcugentbot
Copy link
Copy Markdown

Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1827/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.

@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 11, 2016

retested with all existing MVAPICH2 easyconfigs through easybuilders/easybuild-easyconfigs#2707, good to go, thanks @besserox!

boegel added a commit that referenced this pull request Mar 11, 2016
Remove deprecated configure options from MVAPICH2 easyblock
@boegel boegel merged commit 0416fcb into easybuilders:develop Mar 11, 2016
@besserox besserox deleted the cleanup_mvapich2 branch March 16, 2016 14:14
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.

3 participants