Skip to content

allow setting optarch compiler flags in the easyconfig#2595

Merged
boegel merged 7 commits intoeasybuilders:developfrom
migueldiascosta:optarch_string_from_ec
Nov 22, 2018
Merged

allow setting optarch compiler flags in the easyconfig#2595
boegel merged 7 commits intoeasybuilders:developfrom
migueldiascosta:optarch_string_from_ec

Conversation

@migueldiascosta
Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta commented Sep 27, 2018

If toolchainopts[optarch] from the easyconfig is a string, use that as architecture optimization flags instead of build_options('optarch')

One problem with this approach is that the per-compiler syntax is not supported, unlike build_option('optarch') (which has been parsed earlier, in tools.options._postprocess_optarch)

Using only compiler-specific flags in the easyconfig optarch would not usually be an issue, since the compiler is already set by the easyconfig and this is meant for very specific cases, but I suppose it will break --try-toolchain...

Should we also postprocess optarch here? (or is there a better way?)

Comment thread easybuild/tools/toolchain/compiler.py Outdated
Comment thread easybuild/tools/toolchain/compiler.py Outdated
@boegel
Copy link
Copy Markdown
Member

boegel commented Sep 27, 2018

@migueldiascosta That's a good point, but on the other hand it would be weird to specify flags specific to a compiler different from the toolchain compiler in the easyconfig file...

It's nice to support this for flexibility, but this should really only be used by exception I think, of for site customization reasons.

And --try-toolchain is named 'try' on purpose, there's lots of other things that can go wrong there already (e.g. compiler-specific patches or configopts, etc.).

@damianam Thoughts on this?
Maybe also @bartoldeman who frequently uses --try-toolchain I think...

@boegel boegel added this to the next release milestone Oct 4, 2018
@damianam
Copy link
Copy Markdown
Member

IMO if this doesn't break anything it can go in. I agree that supporting the per-compiler syntax is weird in easyconfigs. It will break --try-toolchain, but so do other things (per compiler patches, environment variables that depend on the particular MPI used, etc)

Comment thread easybuild/tools/toolchain/compiler.py Outdated
Comment thread easybuild/tools/toolchain/compiler.py Outdated
@damianam damianam modified the milestones: next release, 3.8.x Oct 11, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 16, 2018

I'd love to see a test for this before merging it in... Up for that @migueldiascosta? See test/framework/toolchain.py

@migueldiascosta
Copy link
Copy Markdown
Member Author

but I probably should have used a different toolchain for the new test easyconfigs...

@boegel boegel modified the milestones: 3.8.x, next release Oct 19, 2018
@easybuilders easybuilders deleted a comment from boegelbot Nov 20, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 20, 2018

@migueldiascosta Is this still WIP?

Don't worry too much about which toolchains were used in the test easyconfigs, I'm working on a big cleanup there anyway because of impending deprecation of the ictce and goolf toolchains...

@migueldiascosta migueldiascosta changed the title allow setting optarch compiler flags in the easyconfig (WIP) allow setting optarch compiler flags in the easyconfig Nov 21, 2018
… than adding two new test easyconfigs with just a different value for toolchainopts
Comment thread test/framework/easyconfig.py Outdated
res = [os.path.basename(x) for x in find_related_easyconfigs(test_easyconfigs, ec)]
self.assertEqual(res, ['toy-0.0-gompi-1.3.12-test.eb', 'toy-0.0-gompi-1.3.12.eb'])
self.assertEqual(res, ['toy-0.0-gompi-1.3.12-optarch-map.eb', 'toy-0.0-gompi-1.3.12-optarch.eb',
'toy-0.0-gompi-1.3.12-test.eb', 'toy-0.0-gompi-1.3.12.eb'])
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.

@migueldiascosta I would like to avoid adding additional test easyconfigs if we can avoid it, alternate approach implemented in migueldiascosta#5

tweak existing toy easyconfig in test_easyconfig_optarch_flags rather than adding two new test easyconfigs with just a different value for toolchainopts
@boegel boegel dismissed damianam’s stale review November 22, 2018 07:44

remarks handled

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 22, 2018

Thanks @migueldiascosta!

@boegel boegel merged commit 5bfb2fa into easybuilders:develop Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants