Skip to content

V4 header install#614

Closed
lucjaulmes wants to merge 3 commits intoQuEST-Kit:mainfrom
lucjaulmes:v4-header-install
Closed

V4 header install#614
lucjaulmes wants to merge 3 commits intoQuEST-Kit:mainfrom
lucjaulmes:v4-header-install

Conversation

@lucjaulmes
Copy link
Contributor

Fixes #611 header issues:

  • Moves the install directive for headers into main CMakeLists to make it recognize the install prefixes correctly
  • Create a more standard layout with quest.h and other headers under quest/ subdirectory all in the usual include path.
  • Use the build directories to create a structure similar to the desired install directories so that include paths are unique.
  • Make quest.h configured so it contains the pre-processor defines if VERBOSE_LIB_NAME is false, and make those target defines private so they don’t appear in exported cmake files.
  • Do not set the defines in quest.h and keep them in the exported cmake if VERBOSE_LIB_NAME is true.
  • Differentiate exported cmake files using $LIB_NAME as well, so the matching cmake files link to the right verbosely-named libraries and provide the right compile-time flags, with a single set of header files.

I think all of the CMakeFiles machinery (in particular foreach() which seems to be discouraged) can be simplified at the cost of having all the files already in the final layout in the source directory. So all headers at quest/include/quest.h.in and quest/include/quest/*.h respectively. I can update the PR that way if that’s preferred.

lucjaulmes added 3 commits May 6, 2025 17:38
We want <include-path>/quest.h and <include-path>/quest/<header>.h in
final installed structure, so update files accordingly.

This is particularly important as we plan on installing quest.h, but
quest.h is also included by the local code, so it needs to use paths
that are both source-tree layout and install-tree layout.
Use build directory to “stage” includes at install position
@TysonRayJones
Copy link
Member

TysonRayJones commented May 6, 2025

Hi Luc,

Thanks very much for the PR! I like your approach, though am still investigating the problem.

If possible, I'd wish to avoid changing the hundreds of include paths. I've so far been using a variant of Kolpackov's structure, using the include path to semantically separate what's frontend and backend. Consider the includes in accelerator.cpp:

#include "quest/include/types.h"
#include "quest/include/qureg.h"
#include "quest/include/paulis.h"
#include "quest/include/matrices.h"

#include "quest/src/core/accelerator.hpp"
#include "quest/src/core/errors.hpp"
#include "quest/src/core/memory.hpp"
#include "quest/src/core/bitwise.hpp"
#include "quest/src/cpu/cpu_config.hpp"
#include "quest/src/gpu/gpu_config.hpp"
#include "quest/src/cpu/cpu_subroutines.hpp"
#include "quest/src/gpu/gpu_subroutines.hpp"

#include <vector>
#include <algorithm>

A change like this PR's then seems to mislead about the location of the headers

#include "quest/types.h"
...

#include "quest/src/core/accelerator.hpp"
...

It also creates an asymmetry in the working directory of frontend and backend includes within a backend file.

No need to explore this further yourself, I'll tinker when I get a chance to see if the existing paths can be preserved without being terribly non-standard and hacky.

Finally, note the PR should point to devel rather than main 🙏

@lucjaulmes
Copy link
Contributor Author

Sure, no worries! I’ll close this down as it’s pointing to the wrong branch. Just thought I’d share because this is what I’m using currently to run v4 and install different versions to different prefixes on the same machine.

It seems to me though that the structure you’re referring to recommends includes of the type quest/types.h as well as stripping quest/src/ from the private includes (and having $CMAKE_SOURCE_DIR/quest/src/ added to CMake include paths instead of the $CMAKE_SOURCE_DIR). But that’s even more include path changing.

You should be able install to headers as $prefix/include/quest/include/types.h (just a bit odd and non-standard paths), and then create a quest.h.in that works from $CMAKE_BINARY_DIR/include for minimal changes in all layout and includes. In fact I don’t think it would take too much time for me to adjust this PR that way if you’d want to see it.

@lucjaulmes lucjaulmes closed this May 7, 2025
@otbrown
Copy link
Collaborator

otbrown commented May 7, 2025

I see Tyson has beat me to it, but chiming in to say thanks for this @lucjaulmes! Especially for solving the headache of leaking compile definitions, which I'd been sitting on until I had more time.

Apologies for not picking up the install bug sooner, but as ever it worked on my machine workflow! 😅

RE: foreach, I think that and a few other issues could be solved using file sets, except we would need to bump to CMake 3.23. I can look at that later, no need to do it all once in this PR.


Anddddd I was still too slow 😆 @lucjaulmes yes, if you have time to make the revised PR to devel that would be appreciated, but no worries if not!

@lucjaulmes lucjaulmes deleted the v4-header-install branch May 7, 2025 09:24
@lucjaulmes lucjaulmes restored the v4-header-install branch May 7, 2025 09:25
@lucjaulmes lucjaulmes deleted the v4-header-install branch May 8, 2025 16:59
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.

Installing headers

3 participants