[gmx-developers] hidden compiler flags in 4.6?

[Mark Abraham]

The missing documentation is addressed in
https://gerrit.gromacs.org/#/c/2172/. Review will be welcome :-)

Mark

On Sat, Feb 9, 2013 at 12:02 AM, Mark Abraham <mark.j.abraham at gmail.com>wrote:

>
>
> On Fri, Feb 8, 2013 at 9:58 PM, Szilárd Páll <szilard.pall at cbr.su.se>wrote:
>
>>
>> On Fri, Feb 8, 2013 at 5:34 PM, Mark Abraham <mark.j.abraham at gmail.com>wrote:
>>
>>>
>>>
>>> On Tue, Feb 5, 2013 at 8:58 PM, Szilárd Páll <szilard.pall at cbr.su.se>wrote:
>>>
>>>>
>>>> Note that I'm not talking about some magic override, but simply the
>>>> ability to go to the flags and remove e.g. the "-ip" icc flag which I know
>>>> that it can in some cases cause performance regression.
>>>>
>>>
>>> In a fresh release-4-6 build tree, if you use
>>>
>>> CC=icc cmake .. -DGMXC_CFLAGS_RELEASE="-no-ip"
>>>
>>> you get
>>>
>>> cd /nethome/mark/git/another-r46/build-cmake-amd2/src/gmxlib &&
>>> /opt/intel/composer_xe_2011_sp1.9.293/bin/intel64/icc  -Dgmx_EXPORTS
>>> -DTMPI_SET_AFFINITY -DHAVE_CONFIG_H -DHAVE_RDTSCP -DTMPI_EXPORTS -msse2
>>> -std=gnu99 -Wall   -ip -funroll-all-loops -no-ip -O3 -DNDEBUG -fPIC
>>> -I/nethome/mark/git/another-r46/build-cmake-amd2/src
>>> -I/nethome/mark/git/another-r46/build-cmake-amd2/include
>>> -I/nethome/mark/git/another-r46/include
>>> -I/nethome/mark/git/another-r46/src/gmxlib   -openmp -o
>>> CMakeFiles/gmx.dir/rmpbc.c.o   -c
>>> /nethome/mark/git/another-r46/src/gmxlib/rmpbc.c
>>>
>>> which seems to do roughly what you want, from a one-shot, because we
>>> have partially segmented the sets of flags GROMACS adds, and prepend them
>>> for each run of cmake. In this case, GMXC_CFLAGS_RELEASE is the magic name
>>> for the C flags we add for the Release build type. Because we prepend the
>>> flags to the initial value (which is initially unset by GROMACS), the value
>>> passed in from the command line is there at the end. Then, because GROMACS
>>> no longer caches the flags we generate, things can be changed later.
>>>
>>
>> Thanks, that indeed seems to be a decent workaround for my very specific
>> example. Unfortunately, this won't work in general. It will only work if
>> the user wants to remove/inactivate a GROMACS-default compiler flag which
>> happens to have a -no-FOO pair.
>>
>
> ...and works when adding a new flag. What use cases are there that are not
> * inactivating a current flag, or
> * replacing a current flag, or
> * activating a new one?
> Of those, what can't be solved by generating all the flags and doing a
> wholesale replacement with GMX_SKIP_DEFAULT_FLAGS?
>
> The biggest problem I can come up with is needing to remove a flag for
> which there is no negation option (which itself sounds like a serious
> defect in a compiler command line syntax, given the prevailing behaviour of
> gcc and icc as I understand them).
>
> It would be good to document this, but as it only works in a corner case
>> I'm not sure it's worth it.
>>
>>
>>>
>>> Note that CMake caches that command-line initial value itself, so that
>>> the next call to cmake will also result in -no-ip. Then when you want some
>>> other set of flags, without touching the cache you can do
>>>
>>> CC=icc cmake .. -DGMXC_CFLAGS_RELEASE=""
>>>
>>> and this reverts to the out-of-the-box functionality (or you could try a
>>> new set of flags if you wanted).
>>>
>>> Your original example where -m and -x conflict and lead to warnings is
>>> irritating, but not a deal-breaker. Using GMX_SKIP_DEFAULT_CFLAGS provides
>>> a two-step approach to fixing that. Obviously, make knows how to tell the
>>> compiler the right things to do (e.g. src/buildinfo.h has the full CFLAGS
>>> in it), so a *make* target that printed out the CFLAGS (whole and/or in
>>> parts) might be a useful thing.
>>>
>>
>> Sure, that's all clear to me, except that the "two step approach"
>> requires more like 4 steps :)  Additionally it is really easy to screw up
>> for the aforementioned reasons (see  the details on the risk of unknowingly
>> removing -O3).
>>
>
> You can't have a one-step solution for modifying default flags without
> having either
> * a million options for all possible flags (and one of my attempts to fix
> #1038 was criticised for having too many options -
> https://gerrit.gromacs.org/#/c/1908/)
> * knowing in advance what the defaults are
>
> What's your fourth step? :-)
> 1. cmake ..
> 2. Find out what the default flags were - maybe automatically if there's a
> way to get cmake to do it usefully
> 3. cmake .. -DGMX_SKIP_DEFAULT_FLAGS=on -DCMAKE_C_FLAGS="blah"
> 4. Profit???
>
>  There's currently no way to say "cmake
>>> -DUSE_MY_FLAGS_ONLY_IM_SUPER_SMART=-blahblah". I'm not philosophically
>>> opposed to having one, but since compilers normally act on the last of a
>>> series of inconsistent flags, the above approaches seem workable if we
>>> document them in an advanced section of the installation guide. I think
>>> there are probably more not-competent-enough users who'd wrongly try to use
>>> it (or use it wrongly) and run into problems than smart-enough users/admins
>>> with a real need case.
>>>
>>
>> That's exactly why I'm not certain that this workaround is worth
>> documenting. The  problem is that the above approach solves only a special
>> case:
>> - will only work for removing a -FOO for which there is a "-no-FOO" *and*
>> - the compiler used has the policy of silently overriding flags.
>>
>
> As I said above, the approach for appending user flags is more general
> than you have so far recognized. Changing our build system to perhaps
> support warning-free building for "niche market" build cases seems like a
> poor return on our time.
>
> Not only that there is no way to set a "USE_MY_FLAGS_ONLY_IM_SUPER_SMART"
>> variable, but the GMX_SKIP_DEFAULT_CFLAGS workflow is IMO too complicated
>> and fragile (ref:-O3 issue) even for advanced users.
>>
>
> Even an advanced user has to find out what the flags are before they
> attempt to modify them. Any procedure that lets them do that is workable.
> Even just documenting "look at src/buildinfo.h" is technically a solution
> for finding out the compiler flags.
>
> Is there anything you want to be able to do that these kinds of workflows
>>> do not support?
>>>
>>
>> Uhmm, I guess I should clarify this. I am not trying to solve some
>> burning issue that has suddenly crippled me. All my examples were simply
>> use-cases meant to illustrate real-world scenarios in which the new
>> behavior of the build system causes issues.
>>
>> So let me state it again: I am trying to discuss usability concerns of
>> our build system that affect the type B users (i.e people familiar with the
>> concept of compiler flags, comipler optimization, CFLAGS, etc).
>>
>
> Sure, but there's so far no example of a problem/compiler for which some
> solution exists using the current mechanisms (if they were documented).
> Half the exotic compilers people might try are going to perform badly and
> we'll tell them to go and use [ig]cc anyway...
>
>  In my eyes, reviewing and changing flags is much more important than the
>>>> consistency of acceleration flags across cmake reruns, that's why I find it
>>>> unfortunate that the build system got crippled in an attempt to fix that
>>>> issue. Of course, I am biased, but putting aside the fact that I will
>>>> suffer from this build system "behaviour regression", I think many
>>>> sysadmins and other advanced users will also be scratching their heads.
>>>>
>>>
>>> If so, gmx-users must have some filters on it ;-)
>>>
>>
>> I hope you are not suggesting that:
>> "not (exists complaints) => not (exists issues)"
>>
>
> No, but if they're not important enough to write an email about, then
> they're not important enough for us to pre-emptively solve.
>
> GROMACS 4.6 has been out for just a few weeks and I am fairly certain that
>> many have not even gotten as far as installing it.
>>
>
> OK, but we still aren't going to prioritize solving non-yet-demonstrated
> problems over the ones we know about :-)
>
>
>> Moreover, most of the potential issues do not manifest as a catastrophic
>> failures, but are rather silent.
>>
>
> Sure, but other than making it easy to see the compiler flags (which you
> can do with make VERBOSE=1, also) what are you suggesting is a problem we
> can solve?
>
>  Additionally, the last few years I've had the opportunity to be in touch
>> with sysadmins, programmers/engineers of HPC
>> companies, experienced computational scientists, and similar advanced users
>> and have seen their CMake cache and log files. These quite often showed
>> signs of heavy compiler flag customization. Based on this experience, I can
>> safely state that, this category of users does exists, and they do the kind
>> of things that can be problematic with the new build system. Although they
>> might not be great in numbers, I think I still it is not the best idea to
>> ignore them, right?
>>
>
> Sure - but if we're considering something like a monthly patch cycle, if
> there's something that's proven to be fixable, we can fix it.
>
> We can't ignore the possibility that your past correspondents were working
> around our old practice of caching CFLAG variables prematurely, or that
> they wouldn't just go for our GMX_SKIP_DEFAULT_CFLAGS solution even if we
> had something more exotic available.
>
>     and adding or overriding should be possible through a single
>>>>>> command-line invocation.
>>>>>>
>>>>>> More concretely IMO what we need is:
>>>>>> - A summary printed or at least some read-only advanced variables so
>>>>>> that one does not need to run e.g "mdrun -version" to know what flags are
>>>>>> used;
>>>>>>
>>>>> I was (and still am) all for the summary. I really liked Teemu's start
>>>>> of it. But it got cut because of time. Not sure it would be a good idea to
>>>>> print just the flags without having a full summary. I think having
>>>>> read-only variables is very confusing.
>>>>>
>>>>
>>>> I see your point, but you have yet to reply to my criticism on the fact
>>>> that *it is impossible to at least check what flags are being used*and that is a major flaw.
>>>>
>>>
>>> CMake printing a summary seems awkward to me. If it's written as a
>>> status message, you can't see it with ccmake, and if it's written as a
>>> warning ccmake users will get in the habit of skipping real warnings. If
>>> using cmake, we'd want logic to only print such summary information for the
>>> final iteration - not sure if that's possible/easy enough to do.
>>>
>>
>> Printing a summary after the first time configuring succeeds should be
>> quite simple do do - that's how the GPU-related notes are issued when GPUs
>> are detected in the build machine but CUDA is not found.
>>
>
> Yeah, but that's not a sound solution if/when we have further
> configuration that might scroll it off screen, and not a solution at all
> for ccmake, as above.
>
> However, a make target to print the configuration seems to address most of
>>> the issue. It replaces some of the use cases for inspecting config.log one
>>> might have with autoconf build systems. Yes, you'd have to run a normal
>>> cmake to find out the default (whether by a CMake summary or this make
>>> target I propose), then decide how to modify it, then apply it using some
>>> of the above mechanisms. If you magically know already what the defaults
>>> will be, you can skip the first stage already and jump straight to using
>>> GMX_SKIP_DEFAULT_CFLAGS. What do you think?
>>>
>>
>> I'd say we should print a configuration summary (with C/CXX/NVCC flags
>> and the most important other options like fftw, CUDA, etc) both after the
>> configure succeeds the first time as well as on user request.
>>
>> I think a target is a good idea, but I'm wondering whether a target is it
>> more intuitive and/or easier to implement than having a cmake option that
>> the user can turn on to get the summary printed.
>>
>> To check the configuration on certain build directory which one of the
>> following would you prefer?
>>
>> $ make print-config-summary
>>
>> $ cmake -DGMX_PRINT_CONFIG_SUMMARY
>>
>
> The former, because auto-complete will type for me.
>
>
>>     - An intuitive and consistent way to add/change the flags, what the
>>>>>> user wants is simple, but currently requires a set of strange steps: enable
>>>>>> skipping GROMACS flags, copy the printed flags, remove cache, re-run cmake
>>>>>> with the copied flags.
>>>>>>
>>>>> No need to remove the cache.
>>>>>
>>>>
>>>> Fine, remove that step, but you've to add two more instead:
>>>> - check default CMake flags;
>>>> - re-add those when setting flags manually
>>>>
>>>> Again, I would very much appreciate of you or others developers would
>>>> actually comment on the issue in question: the usability concerns around
>>>> this rather long, complicated, and error-prone set of steps required just
>>>> to change a flag -Foo to -Bar.
>>>>
>>>
>>> I was turned off by the tone of the original email. I imagine that has
>>> something to do with the low response rate so far. We've all been wrong at
>>> some point. Let's
>>>
>>
>> I see you criticism and I was indeed a bit pissed in the beginning. Sorry
>> for the rather pushy mail.
>>
>> However, I still think that this major change in the build system
>> behavior was merged in without much discussion, review, or feedback. The
>> same is true for the closing of related bugs - if not both, at least 1040.
>>
>
> #1038 was up for several months. My RFC for fixing it (
> https://gerrit.gromacs.org/#/c/1908) was up for a month, and
> unfortunately you didn't manage to comment on it. Roland's solution for
> #1038 addressed the issue squarely in the context of that lengthy
> discussion, and passed our normal standards of code review. So I took my
> original attempt down. No problem there.
>
> #1040 was posted by you and also up for several months. Roland's solution
> permits some of the operations you requested (one-shot command lines, and
> adding a compiler option without cache editing) though we still need to
> document how/when to use it.
>
> Your other suggestion in #1040 requires us to somehow override CMake's
> default behaviour, as noted by Roland there. Changing that would require
> significant discussion, and it was entirely appropriate not to do that at
> all. I agree that this behaviour of CMake makes it easy for the user to do
> the wrong thing, but that is also something we can document. 'Fixing" it
> might leave us vulnerable in the future to some change in CMake's
> behaviour. So maybe the Redmine issue was closed prematurely, but the code
> passed the normal review and addressed the fraction of your bug report that
> was readily addressable.
>
> These issues are up for months and you didn't manage to make the time to
> discuss how to solve them. Others had been discussing and trying to solve
> them. They thought the issues were solved and acted upon their consensus.
> So the ball is in your court to show how they are not solved. If you still
> don't have time to do that, then they're still putatively solved :-)
>
> start discussions by asking questions like "is there supposed to be a way
>>> for a user to add/replace/find out about a compiler flag?" If there isn't
>>> (or it isn't a
>>>
>>
>> Again, you're right, I could have been a bit less judgmental. However, as
>> I know our the build system fairly well and I am also somewhat familiar
>> with cmake, I was quite certain that there is no robust workaround the
>> issues raised.
>>
>>
>>> good solution) then discussion can move forward happily to solving the
>>> problem. If there is an existing solution, people are going to be happier
>>> to mention it if they don't feel like they have to defend their previous
>>> actions, or confront the asker by telling them they were wrong :-)
>>>
>>
>> There isn't much to defend here, I think. This was *a* solution to a/some
>> problem(s) which introduced a set of other problems. My tone and wording
>> was not demanding because it was questioning the solution, but because I
>> was questioning:
>> i) whether introducing new issues was really appropriate/necessary;
>> ii) merging and closing bugs hastily without much feedback.
>>
>
> Unfortunately, Redmine doesn't note patches that never hit the history,
> but to suggest there was only limited discussion is simply wrong.
>
> Mark
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20130218/b92acd6f/attachment.html>