Skip to content

Rearrange public API headers#4841

Merged
msimberg merged 2 commits into
TheHPXProject:masterfrom
msimberg:rearrange-api-headers
Jul 22, 2020
Merged

Rearrange public API headers#4841
msimberg merged 2 commits into
TheHPXProject:masterfrom
msimberg:rearrange-api-headers

Conversation

@msimberg
Copy link
Copy Markdown
Contributor

As discussed with @hkaiser and @aurianer. We decided to change hpx/x.hpp headers to include both distributed and local headers wherever it fits (like future.hpp and runtime.hpp, more in the future), and the distributed header is conditionally included depending on if the distributed runtime is enabled in the configuration.

Users that really always only want the local functionality should include hpx/local/x.hpp. Not everything has a header in hpx/local or hpx/distributed as it doesn't make sense for everything. Should we add dummy headers hpx/local/x.hpp/hpx/distributed/x.hpp (that would just include the same as hpx/x.hpp) for some sort of symmetry? (I vote no, since it's just more to maintain and the cases where the split is needed will be listed in the documentation.)

Extra: I moved most of the API headers to the include module where it's a bit easier to have an overview of them.

@msimberg msimberg added this to the 1.5.0 milestone Jul 20, 2020
@msimberg msimberg force-pushed the rearrange-api-headers branch from a23c64e to afa8750 Compare July 21, 2020 13:57
@msimberg msimberg force-pushed the rearrange-api-headers branch from afa8750 to 1109820 Compare July 21, 2020 13:58
@msimberg
Copy link
Copy Markdown
Contributor Author

msimberg commented Jul 21, 2020

Flyby: I had completely forgotten to add most of the synchronization primitives to the API page, now they're there.

Question: Should we already add the trio channel, barrier, latch to the hpx::distributed namespace (and headers)? I see no reason why not. These are most likely some of the more stable distributed LCOs we have. I can't judge how much else from the collectives modules should be added to a non-experimental namespace though.

Edit: I've now added headers for barrier, channel, and latch.

@msimberg msimberg marked this pull request as ready for review July 21, 2020 14:18
@msimberg msimberg force-pushed the rearrange-api-headers branch from 9783f00 to eed6733 Compare July 21, 2020 15:24
@biddisco
Copy link
Copy Markdown
Contributor

I find this a bit strange. Why not have

#include <hpx/x.hpp>

and inside the x.hpp add

#if defined(HPX_HAVE_DISTRIBUTED_RUNTIME)
#include <hpx/distributed/x.hpp>

and not leave it to chance that the user 'knows' every module that has local and distributed versions?

@msimberg
Copy link
Copy Markdown
Contributor Author

I find this a bit strange. Why not have

#include <hpx/x.hpp>

and inside the x.hpp add

#if defined(HPX_HAVE_DISTRIBUTED_RUNTIME)
#include <hpx/distributed/x.hpp>

and not leave it to chance that the user 'knows' every module that has local and distributed versions?

I'm quite honestly unsure if we agree or disagree here. That sounds exactly like what I've done. See e.g. hpx/future.hpp: https://github.com/STEllAR-GROUP/hpx/blob/eed6733f835bfbc19242e419d0aed5832b43c824/libs/include/include/hpx/future.hpp. Are you saying that every hpx/x.hpp should also have a hpx/local/x.hpp and hpx/distributed/x.hpp no matter if there actually is a distinction? The users who don't want to care can include just hpx/x.hpp, and the users who absolutely want to avoid including distributed features would have to (currently) know which headers have a local variant (or they just check in the documentation).

@msimberg msimberg force-pushed the rearrange-api-headers branch from eed6733 to 1353c5d Compare July 22, 2020 08:29
@biddisco
Copy link
Copy Markdown
Contributor

Sorry. I completely misunderstood the PR and got the diffs the wrong way around. Please proceed and ignore my comments.

Copy link
Copy Markdown
Contributor

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@msimberg msimberg merged commit ef3bbcf into TheHPXProject:master Jul 22, 2020
@msimberg msimberg deleted the rearrange-api-headers branch July 22, 2020 14:20
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.

3 participants