-
Notifications
You must be signed in to change notification settings - Fork 699
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
{toolchain} intelcuda 2017b #6709
{toolchain} intelcuda 2017b #6709
Conversation
…b, HPL-2.2-intelcuda-2017b.eb, iccifortcuda-2017.4.196-GCC-6.4.0-2.28.eb, iimpic-2017b.eb, imkl-2017.3.196-iimpic-2017b.eb, impi-2017.3.196-iccifortcuda-2017.4.196-GCC-6.4.0-2.28.eb, intelcuda-2017b.eb
Test report by @vanzod |
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.
lgtm
('CUDA', '9.0.176', '', ('iccifort', intelver)), | ||
('impi', '2017.3.196', '', ('iccifortcuda', intelver)), | ||
('imkl', '2017.3.196', '', ('iimpic', version)), | ||
] |
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.
Any particular reason you did it this way instead of how it looks in intelcuda-2016.10.eb ?
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.
I simply followed the rule of encapsulating whatever gets repeated into a variable after consulting with @boegel
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.
Yeah, but why use
('GCCcore', gccver),
('binutils', binutilsver, ...)
instead of
('iccifort', intelver),
as in intelcuda-2016.10.eb
(That was what my question was really targeting)
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.
@akesandgren The reason why it is different from intelcuda-2016.10.eb
is that I took intel-2017b.eb
as template. I honestly do not know the reason why we prefer to call the separate components instead of the lower toolchain modules. @boegel definitely has better memory than me on that.
That said, I'm fine using the convention you used for the previous intelcuda
.
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.
Ok, I used iccifort in the old one since it made more sense to me at the time, but the GCCcore+binutils like intel-201... uses also makes sense, and keeping it similar to intel.eb makes a lot of sense.
Keep it.
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.
There's actually no point in also including GCCcore
and binutils
in the dependencies
list for intel*
toolchains, other than to clarify that GCCcore
+ binutils
is used as a base. There's no technical reason for it, everything will work just fine without it.
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
Going in, thanks @vanzod! |
(created using
eb --new-pr
)