Adios2: install: internal headers not placed under adios2/

Created on 6 Jun 2019  Â·  18Comments  Â·  Source: ornladios/ADIOS2

As of commit ec59c81e3d on master, I see a layout in the install tree like this:

include/adios2.h
include/adios2_c.h
include/adios2/ADIOS*.h
include/c/*.h
include/cxx11/*.h

That means paths to files like cxx11/Variable.h do not mention "adios2" in them anywhere.
Instead the layout should be something like

include/adios2.h
include/adios2_c.h
include/adios2/ADIOS*.h
include/adios2/c/*.h
include/adios2/cxx11/*.h

Cc: @chuckatkins

work-in-progress medium

Most helpful comment

@willliamfg I'll get the header shuffling in for the 2.4 release since we definitely want that fixed if it's going to be widely deployed as a module. As for the communication rework, the end of the month might be a little tight since we'll need to have sufficient testing on the big machines before merging. It might be done in time but I don't think we can guarantee that. The result should be only a single build that can do both.

All 18 comments

@bradking just curious, what's the advantage for the proposed layout?

E.g. if a package manager installs ADIOS2 to the /usr prefix then the path /usr/include/cxx11/Variable.h does not mention adios at all.

@bradking thanks, I thought we were following the MPI/openmpi way. At least in my Ubuntu 18 box openmpi headers have the mpi prefix /usr/include/mpi/ which is a soft link to /usr/lib/x86_64-linux-gnu/openmpi/include. I'd expect cmake to look under /usr/include/adios2/ for public adios2 headers. I have never installed adios2 under /usr....can you verify it adds the adios2 prefix to the public headers?

ls /usr/include shows there are many packages doing the above (adding a prefix). I can only speak for my Ubuntu box, haven't used other Linux distros, though.

Traditionally, libraries would install their header(s) directly into /usr/include, or /usr/local/include, basically, ${CMAKE_INSTALL_PREFIX}/include. That's fine for libraries with one or a few headers, but it clutters thing up and potentially conflicts with other libraries' headers. For example, hdf5 puts hdf5.h, hdf5_hl.h but also a whole bunch of H5*.h headers directly into the include/ directory. It'd be nicer of those headers were in a subdirectory, but at least by means of the H5 prefix it's fairly clear that they belong to hdf5 and aren't likely to conflict.

It's true that, e.g., the adios2 cxx11/*.h headers aren't obviously related to adios2, and as such there could also be conflicts with other libraries, so it would be cleaner to move all headers into the adios2/ subdirectory. adios2.h and adios2_c.h should remain in the top-level include/ to maintain compatibility, as those are the public interface, and they're also named in a way that it's obvious which library they belong to.

Other than the c/* and cxx11/* headers that I'd support moving, there also a bunch of scattered headers

cm_transport.h  cxx11       enet        evpath.h    fm.h
atl.h       cm_schedule.h   cod.h       dill.h      ev_dfg.h    ffs.h       revpath.h

Those are installed as a side effect of building adios2, I guess, as they're thirdparty/ dependencies. I'm not sure what (if anything) should be done about them.

All of this doesn't preclude users from adding their own prefix, ie, instead of installing into /usr/local/{include,lib,...}, they might want to install into /usr/local/pkg/adios2-2.4.0rc1/{include,lib,...}, as is definitely needed if you want to keep multiple versions/archs around. But that's an orthogonal issue which is handled by CMAKE_INSTALL_PREFIX.

adios2 headers directly under /usr/include mean than MPI and truly non-MPI versions can't be supported at the same time. Can this be resolved by adding a piece of documentation saying: always use a prefix?

Probably there is no need for both packages on a single system where a user
installs adios under /usr
The user either builds it with MPI and thus have both MPI and non-MPI in
one big package, or he/she doesn't and then have a non-MPI package on the
system.

On Thu, Jun 6, 2019 at 4:10 PM William F Godoy notifications@github.com
wrote:

adios2 headers directly under /usr/include mean than MPI and truly
non-MPI versions can't be supported at the same time. Can this be resolved
by adding a piece of documentation saying: always use a prefix?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ornladios/ADIOS2/issues/1477?email_source=notifications&email_token=AAYYYLOPV4M3HPWQLMWS2RTPZFVJTA5CNFSM4HVGKYQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXEAOTA#issuecomment-499648332,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAYYYLMJPMQ5VXYDB4BIPJTPZFVJTANCNFSM4HVGKYQA
.

adios2 headers directly under /usr/include mean than MPI and truly non-MPI versions can't be supported at the same time. Can this be resolved by adding a piece of documentation saying: always use a prefix?

That's true, but this is just how it is for any package. You cannot have two different instances of the same package installed under the same prefix. That's true for having different versions of the same package, for having it built for different archs, and for differently configured instances of the same package, as with MPI + no-MPI (just as with bzip2 support vs w/o bzip2 support).

With adios2, with the recent work on supporting serial use within the parallel adios2, it hopefully won't be needed to install a separate serial version anymore in most cases.

@bradking is in the middle of re factoring, encapsulating, and abstracting the communication code such that there will be no need for separate libraries. I believe we can come up with a way to even eliminate the need at link time, but that's a different problem. The includes really do need to be encapsulated into a prefix in the include. I'm happy to take that one and do the file re-org.

@bradking, @chuckatkins: There may be a duplication of effort here. @pnorbert and myself just did work which provides the serial adios2 API from the parallel adios2 library (ie, built with MPI). We ended up choosing the minimal route, which avoids introducing an entirely new API over the familiar MPI. MPI still needs to be always linked, though with modern tools / shared libs that's basically transparent. I don't really see a way how one would avoid that, other than doing dynamic loading of the MPI library on demand, which doesn't sound like fun...

Not duplicating, @bradking is starting from where you left off. The big advantage of the encapsulation and abstraction is that it allows for non mpi based communication backends to be used, provided a similar enough set of semantics and patterns can be achieved. This will be Important for the long term sustainability of adios and it's adaptability other platforms, frameworks, and io patterns. It will also resolve some long-standing issues related to communicator lifetime and leakage.

All that is of course way outside the scope of this issue, which is that the headers really need to be pulled under the adios2 directory in install.

Sure, I definitely agree with moving the headers.

Just to give an outline of what I'm still intending to do: I think rather than switching based on MPI_Initialized and !MPI_Finalized, it's cleaner to do so by using a designated communicator, and MPI_COMM_NULL fits the bill perfectly. That'll avoid some subtleties which still exist in the current approach. But that should basically be orthogonal to a bigger picture communication abstraction layer (because it's really about the serial case, ie, NO communication ;).

And then I might also go and suggest dropping all of the mpidummy MPI_File stuff, because it's currently unused. And there's already a file abstraction (Transport) layer, which means if one were to offer a MPI_File based method at some point in the future, it really only makes sense to offer that Transport if MPI is actually available.

I like all of the above. What's the target for the refactoring? Is it v2.4 at the end of this month? Is the goal a "superbuild" of a mixed MPI and non-MPI builds? Can we add a "Milestone" tag? Thank you all.

@willliamfg I'll get the header shuffling in for the 2.4 release since we definitely want that fixed if it's going to be widely deployed as a module. As for the communication rework, the end of the month might be a little tight since we'll need to have sufficient testing on the big machines before merging. It might be done in time but I don't think we can guarantee that. The result should be only a single build that can do both.

@chuckatkins thanks, makes sense.

@KyleFromKitware I think a good organization to end up with would be something like:

include/adios2.h          # The "entry point header" for the C++ bindings
include/adios2_c.h        # The "entry point header" for the C bindings
include/adios2/common/... # Common headers used by both (ADIOS2Config.h, etc.)
include/adios2/CXX11/...  # The C++11 headers
include/adios2/C/...      # The C headers

1510 almost got it all but there are still two remaining pieces:

  • the fortran module s are still getting put into include/fortran and need to be moved into include/adios2/fortran. Note that this will also require adjusting the install interface include dirs on the adios2_f fortran target
  • Some third party headers from /thirdparty seem to be seeping into the install and shouldn't be there as they are strictly internal private dependencies.
Was this page helpful?
0 / 5 - 0 ratings

Related issues

ax3l picture ax3l  Â·  6Comments

AaronV77 picture AaronV77  Â·  6Comments

ax3l picture ax3l  Â·  8Comments

tleger26 picture tleger26  Â·  6Comments

ax3l picture ax3l  Â·  4Comments