Skip to content

fix HierarchicalMNS for cgoolf and goolfc toolchains + add iompi definition#1049

Merged
boegel merged 7 commits intoeasybuilders:v1.15.xfrom
boegel:hmns_goolfc_cgoolf
Sep 23, 2014
Merged

fix HierarchicalMNS for cgoolf and goolfc toolchains + add iompi definition#1049
boegel merged 7 commits intoeasybuilders:v1.15.xfrom
boegel:hmns_goolfc_cgoolf

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Sep 23, 2014

This enables making HierarchicalMNS compatible with the goolfc, cgoolf and iomkl toolchain (see also easybuilders/easybuild-easyconfigs#1099 w.r.t. iomkl)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please come up with a better and more maintnable solution. like a dict/map module constant.

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 don't think that can work... It might for only the name, but we need to determine the version part too, which can't be easily pushed into a dict imho...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the sanity check if tc_comps[0]['version'] == tc_comps[1]['version'] not, that you'll have to double verify. but the rest could be done without any issue like

MAP = {
    ['Clang', 'GCC']: "ClangGCC",
    ...
}

# saniutycheck MAP
set_map = dict([(set(x),(x,v)) for x,v in MAP.items])
if not len(MAP) == len(set_map): raise something
ordered_comp_names, tc_comp_name = set_map[set(comp_names))]
tc_comp_ver='-',join(map(comp_version.get, ordered_comp_names))
# do something to sanity check intel and fix intel version

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and deal with the KeyError 😄

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.

@stdweird: remark fixed, it's doable after all...

@wpoely86
Copy link
Copy Markdown
Member

Looks good

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make it a class constant (will make it a lot easier to create local HMNS) (and indicate in comment that the key is sorted)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Sep 23, 2014

@stdweird: remarks fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make this a single test? tc_comp_name == 'intel' and comp_versions['icc'] != comp_versions['ifort']?

@stdweird
Copy link
Copy Markdown
Contributor

minor remark, otherwsie ready to merge

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Sep 23, 2014

Thanks for the review @stdweird and @wpoely86!

boegel added a commit that referenced this pull request Sep 23, 2014
fix HierarchicalMNS for cgoolf and goolfc toolchains + add iompi definition
@boegel boegel merged commit 7861f6f into easybuilders:v1.15.x Sep 23, 2014
@boegel boegel deleted the hmns_goolfc_cgoolf branch September 23, 2014 19:49
boegel added a commit to boegel/easybuild-framework that referenced this pull request Sep 24, 2014
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