search

celerity / celerity-runtime

High-level C++ for Accelerator Clusters
https://celerity.github.io
MIT License
139 stars 18 forks source link

Compatibility with AdaptiveCpp versioning #218

Closed fknorr closed 11 months ago

fknorr commented 11 months ago

With the re-naming to AdaptiveCpp hipSYCL has adopted a new versioning scheme. We have fixed the version to hipSYCL 0.9.1 so far which now breaks building with AdaptiveCpp. This PR removes the version check altogether.

If Acpp ends up with some form of API stability around its versioning scheme we can think about re-adding version constraints in the future.

github-actions[bot] commented 11 months ago

Check-perf-impact results: (5a19ced85f862a00d0114dd241122462)

:question: No new benchmark data submitted. :question:
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

illuhad commented 11 months ago

Can you elaborate on what the problem is? It's not clear to me how a version like 23.10.0 is different than if we had stuck to e.g. 0.9.5. In both cases the number is strictly greater than 0.9.1. There was no intent of causing a break, so I'm curious.

fknorr commented 11 months ago

I did not look into this in more depth, but before the renaming, 

find_package(hipSYCL "0.9.1" CONFIG REQUIRED)

was compatible with the latest develop (even though 0.9.4 had been released in the meantime), but with the new versioning scheme CMake will reject this line with

CMake Error at CMakeLists.txt:48 (find_package):
  Could not find a configuration file for package "hipSYCL" that is
  compatible with requested version "0.9.1".

  The following configuration files were considered but not accepted:

    /opt/hipsycl/lib/cmake/hipSYCL/hipsycl-config.cmake, version: 23.10.0

Celerity uses commit ranges instead of version ranges to specify compatibility with SYCL implementations atm, so we can consider this a fix on our side rather than a workaround :shrug:

illuhad commented 10 months ago

@fknorr I think it might be because we have set compatibility to same major version: https://github.com/AdaptiveCpp/AdaptiveCpp/blob/f577603d89f3b65575b456de084bc0799c886e29/CMakeLists.txt#L459

So 0.9.1, 0.9.2...0.9.9 would be fine, but 23.10.0 is not. We should change that as such a compatibility rule does not make sense for a year.month release scheme.

But good to hear you are not directly impacted and have resolved it otherwise :)