Re: [AMBER] [PATCH 4/8] sqm: Fix OpenMP build

From: Reinis <gam4rei.gmail.com>
Date: Tue, 23 Oct 2012 23:55:26 +0300

On Tue, Oct 23, 2012 at 04:51:43PM -0300, Gustavo Seabra wrote:
> >>>
> >>> $omp_flag expands to -fopenmp -DOPENMP and I tried to add it
> >>> also to FPPFLAGS and there was no difference, still fails. I'm
> >>> also using ifdefs in my patch to isolate OPENMP parts, so it
> >>> does receive that directive.
> >>
> >> Where did you add this? If you add it to the Amber/AmberTools
> >> configure, it is not passed down to the sqm compilation. The
> >> only way to compile sqm with OpenMP support is looking for the
> >> specific calls in the Makefile in the sqm directory, and
> >> altering it there.
> >
> > I added this in AmberTools/src/configure2, that is the only
> > place where it makes sense to modify general flags such as
> > FFLAGS, CFLAGS and CXXFLAGS which influence all of the
> > applications built. In fact configure files are the only ones
> > which should require modifications (if at all) by users (given
> > proper build system).
> >
> > These variables are set by configure and referenced by
> > Makefiles, so every change is propagated to them. And they are
> > propagated since I can make sqm to use any number of threads I
> > want by using omp_get_max_threads().
> >
> > Obviously, your way is not the only way. I think we already
> > established that from the time Ross Walker worked with it the
> > code was changed in a way that it breaks OpenMP build.
>
>
> That's precisely my point. I was trying to understand why it
> didn't work for you, and that's the reason. As Ross explained,
> the configure will not pass the directives to the compilation
> of the OpenMP part of SQM, and that was done on purpose so
> that it can only be compiled by them, while in the development
> process. I suppose the idea is, once they finish and know it
> is working, they would put the right commands in the configure
> script.
>
> If you really want to give it a try, then you have to modify
> the config.h files by hand, but at your own risk.

No, you don't understand it.
- config.h is generated by configure
- configure sets $omp_flag to "-fopenmp -DOPENMP"
- that is used to to set FFLAGS=$omp_flag
- which expands to FFLAGS="-fopenmp -DOPENMP"
- there is nothing special about -DOPENMP it is passed along
  with -fopenmp
- Makefile uses $(FC) $(FFLAGS) to compile
- that expands to "gfortran -fopenmp -DOPENMP"
- now -fopenmp is a compiler flag which tells compiler to enable
  OpenMP support in this program
- and -DOPENMP is used by preprocessor for conditional
  compilation of the code included in ifdef code blocks

So the code needed only for OpenMP is written like this:
#ifdef OPENMP
... OpenMP specific code goes here ...
#endif

- preprocessor reads the source code file and checks if OPENMP
  is defined and it is since it has -DOPENMP given to it (all
  options starting with -D are defines)
- preprocessor removes ifdef and endif lines leaving the code in
  between in the file and that is passed for compilation

So there is no need for manually editing config.h. All needed
options are already there.

The patch I sent fixes issues which can not be ameliorated by
any amount of configure and Makefile editing if one wants to
compile sqm with OpenMP support.

The point Ross raised was that I should pass also -DOPENMP
together with -fopenmp, but since I was doing it from the very
beginning he agreed that the code was broken.

And the patch on its own does NOT enable OpenMP support in sqm.
What it does is merely allowing for successful compilation with
such support (even developers need to fix this compilation issue
to be able to work on OpenMP support in sqm in any meaningful
way).


Reinis

P.S. So did my patches where eaten by amber or they reached only
developers, or I was the only one who didn't receive them back?
I have in mailing list settings enabled that my mails are sent
also to me from the list, but I didn't receive any of them. What
happened?

_______________________________________________
AMBER mailing list
AMBER.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber
Received on Tue Oct 23 2012 - 14:00:04 PDT
Custom Search