Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions .github/workflows/ci-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,41 @@ jobs:
- run: tar -czvf build.tar.gz build
if: ${{ matrix.save }}

# Run trace with each blue noise sampler to validate table linkage
- run: ./build/src/tools/cli/trace pmjbn box
- run: ./build/src/tools/cli/trace sobolbn box
- run: ./build/src/tools/cli/trace latticebn box

# Upload tools build for test-generate
- uses: actions/upload-artifact@v4
with: {name: "${{ matrix.build }}-${{ matrix.arch }}", path: build.tar.gz}
if: ${{ matrix.save }}

build-tools-gpu:
name: Build tools library GPU
needs: test-unit
strategy:
matrix:
build: [Release, Debug]
runs-on: ubuntu-20.04-gpu-t4-4c-16g-176h
container:
image: aswf/ci-base:2023
options: --gpus all
steps:
- uses: actions/checkout@v4
- run: cmake --preset unix -D OPENQMC_ARCH_TYPE=GPU -D CMAKE_CUDA_ARCHITECTURES=75 -D CMAKE_BUILD_TYPE=${{ matrix.build }}
- run: cmake --build --preset unix
- run: tar -czvf build.tar.gz build

# Run trace with each blue noise sampler to validate table linkage
- run: ./build/src/tools/cli/trace pmjbn box
- run: ./build/src/tools/cli/trace sobolbn box
- run: ./build/src/tools/cli/trace latticebn box

# Upload tools build for test-generate-gpu
- uses: actions/upload-artifact@v4
with: {name: "${{ matrix.build }}-GPU", path: build.tar.gz}

pre-test-generate:
name: Pre generate reference data
needs: build-tools
Expand Down Expand Up @@ -178,3 +208,29 @@ jobs:
- run: tar -xzvf build.tar.gz
- run: ./build/src/tools/cli/generate ${{ matrix.sampler }} > output
- run: diff output data

test-generate-gpu:
name: Run generate GPU
needs:
- pre-test-generate
- build-tools-gpu
strategy:
matrix:
build: [Release, Debug]
sampler: [pmj, sobol, lattice]
runs-on: ubuntu-20.04-gpu-t4-4c-16g-176h
container:
image: aswf/ci-base:2023
options: --gpus all
steps:
# Download tools build
- uses: actions/download-artifact@v4
with: {name: "${{ matrix.build }}-GPU"}

# Download sample data
- uses: actions/download-artifact@v4
with: {name: "${{ matrix.sampler }}"}

- run: tar -xzvf build.tar.gz
- run: ./build/src/tools/cli/generate ${{ matrix.sampler }} > output
- run: diff output data
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Added

- GPU coverage on the CI pipeline is restored using ASWF hosted T4 runners, building the tools as CUDA and validating device results against CPU reference data.

### Changed

- Migrated the project from C++14 to C++17, moving the minimum supported VFX Reference Platform from CY2018 to CY2021, and the minimum NVCC version from 10 to 11. Blue noise tables are now `inline constexpr`, so header-only installs no longer duplicate table data across compilation units.
- Sped up scalar paths of `oqmc::sobolReversedIndex` using the construction method from Ahmed 2024.

### Deprecated
### Removed

- Removed the binary library variant and its `OPENQMC_ENABLE_BINARY`, `OPENQMC_SHARED_LIB` and `OPENQMC_FORCE_PIC` CMake options. C++17 `inline` variables make the header-only install equivalent, so downstream projects using these options should simply remove them.

### Fixed

- Improve quality of uniform float distribution in `oqmc::uintToFloat`.
Expand Down
3 changes: 0 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ endif()
option(OPENQMC_BUILD_TOOLS "Build the command line tools.")
option(OPENQMC_BUILD_TESTING "Build the unit tests.")
option(OPENQMC_FORCE_DOWNLOAD "Ignore installed dependencies.")
option(OPENQMC_ENABLE_BINARY "Build binary to reduce memory cost.")
option(OPENQMC_SHARED_LIB "Make a shared library, in place of static.")
option(OPENQMC_FORCE_PIC "Force PIC for static libraries.")

# Enable testing

Expand Down
2 changes: 1 addition & 1 deletion CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"inherits": "base",
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_STANDARD": "14",
"CMAKE_CXX_STANDARD": "17",
"CMAKE_CXX_STANDARD_REQUIRED": "ON",
"CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
"OPENQMC_CXX_FLAGS": "-Werror;-Wall;-Wextra;-Wpedantic;-Wno-unknown-pragmas",
Expand Down
30 changes: 8 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ and is actively used in VFX production.

## Description

This C++14 (CPU, GPU) header only library provides an API to deliver high
This C++17 (CPU, GPU) header only library provides an API to deliver high
quality QMC sample points. The API aims to be compatible with a variety of
common use cases found in production codebases. The library also provides
multiple state of the art back-end implementations to minimise noise.
Expand All @@ -52,7 +52,7 @@ Project features are:
- Clear and extendable code base.
- Unit and statistical testing.
- Modern [CMake](https://cmake.org/) based build system.
- Header only and binary configurations.
- Header only implementation.
- No library or STL dependencies.
- Includes tools, docs and examples.

Expand Down Expand Up @@ -125,8 +125,8 @@ more advanced solutions.
## Requirements

<!-- MKDOCS_SPLIT: setup.md -->
The library itself has no dependencies other than C++14. Although not tested
with older versions of GCC, this should make it compatible with CY2018 and
The library itself has no dependencies other than C++17. Although not tested
with older versions of GCC, this should make it compatible with CY2021 and
newer versions of the [VFX Reference Platform](https://vfxplatform.com).

Supported operating systems:
Expand All @@ -146,7 +146,7 @@ Tested compilers:
- Clang 6
- Clang 21
- GCC 7.5
- NVCC 10
- NVCC 11

## Installation

Expand Down Expand Up @@ -188,8 +188,6 @@ fastest and simplest approach.
```cmake
# Optionally set options (they are OFF by default)
set(OPENQMC_ARCH_TYPE AVX CACHE STRING "" FORCE)
set(OPENQMC_ENABLE_BINARY ON CACHE BOOL "" FORCE)
set(OPENQMC_SHARED_LIB ON CACHE BOOL "" FORCE)

# Load external dependencies
add_subdirectory(path/to/submodule EXCLUDE_FROM_ALL)
Expand All @@ -199,9 +197,6 @@ mark_as_advanced(FORCE OPENQMC_ARCH_TYPE)
mark_as_advanced(FORCE OPENQMC_BUILD_TOOLS)
mark_as_advanced(FORCE OPENQMC_BUILD_TESTING)
mark_as_advanced(FORCE OPENQMC_FORCE_DOWNLOAD)
mark_as_advanced(FORCE OPENQMC_ENABLE_BINARY)
mark_as_advanced(FORCE OPENQMC_SHARED_LIB)
mark_as_advanced(FORCE OPENQMC_FORCE_PIC)

# Add dependencies
target_link_libraries(${PROJECT_NAME} PRIVATE OpenQMC::OpenQMC)
Expand All @@ -219,7 +214,7 @@ installation process will follow an idiomatic approach that provides you the
opportunity to set library build options. An example:

```bash
cmake -B build -D CMAKE_INSTALL_PREFIX=/install/path -D OPENQMC_ENABLE_BINARY=ON
cmake -B build -D CMAKE_INSTALL_PREFIX=/install/path
cmake --build build --target install
```

Expand All @@ -245,8 +240,8 @@ method isn't recommended, as it can be error-prone and prevents using build
options otherwise available via CMake.

```cmake
# Enable C++14 support
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_14)
# Enable C++17 support
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_17)

# Add dependencies
target_include_directories(${PROJECT_NAME} PRIVATE path/to/library/include)
Expand All @@ -260,15 +255,6 @@ which apply to downstream projects. The options are:
- `OPENQMC_ARCH_TYPE`: Sets the architecture to optimise for on the CPU, or to
target NVIDIA GPUs. Option values can be `Scalar`, `SSE`, `AVX`, `ARM` or
`GPU`. Default value is `Scalar`.
- `OPENQMC_ENABLE_BINARY`: You can reduce binary size of downstream projects by
opting for a binary variant of the library. Option values can be `ON` or
`OFF`. Default value is `OFF`.
- `OPENQMC_SHARED_LIB`: You can request a shared library instead of the default
static. This also automatically enables PIC. Option values can be `ON` or
`OFF`. Default value is `OFF`.
- `OPENQMC_FORCE_PIC`: When compiling a static library and the downstream
project is a shared library, you can force enable PIC. Option values can be
`ON` or `OFF`. Default value is `OFF`.

## Versioning

Expand Down
4 changes: 2 additions & 2 deletions cmake/examples/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ project(example-include VERSION 0.0.0 LANGUAGES CXX)

add_executable(${PROJECT_NAME} main.cpp)

# Enable C++14 support
# Enable C++17 support

target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_14)
target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_17)

# Add dependencies

Expand Down
48 changes: 6 additions & 42 deletions include/oqmc/bntables.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,82 +79,46 @@ static_assert(xBits == yBits,
namespace pmj
{

#if defined(OQMC_ENABLE_BINARY)

/// Optimised blue noise key table for pmj.
extern const std::uint32_t keyTable[size];

/// Optimised blue noise rank table for pmj.
extern const std::uint32_t rankTable[size];

#else

/// Optimised blue noise key table for pmj.
constexpr std::uint32_t keyTable[] = {
inline constexpr std::uint32_t keyTable[] = {
#include "data/pmj/keys.txt"
};

/// Optimised blue noise rank table for pmj.
constexpr std::uint32_t rankTable[] = {
inline constexpr std::uint32_t rankTable[] = {
#include "data/pmj/ranks.txt"
};

#endif

} // namespace pmj

namespace sobol
{

#if defined(OQMC_ENABLE_BINARY)

/// Optimised blue noise key table for sobol.
extern const std::uint32_t keyTable[size];

/// Optimised blue noise rank table for sobol.
extern const std::uint32_t rankTable[size];

#else

/// Optimised blue noise key table for sobol.
constexpr std::uint32_t keyTable[] = {
inline constexpr std::uint32_t keyTable[] = {
#include "data/sobol/keys.txt"
};

/// Optimised blue noise rank table for sobol.
constexpr std::uint32_t rankTable[] = {
inline constexpr std::uint32_t rankTable[] = {
#include "data/sobol/ranks.txt"
};

#endif

} // namespace sobol

namespace lattice
{

#if defined(OQMC_ENABLE_BINARY)

/// Optimised blue noise key table for lattice.
extern const std::uint32_t keyTable[size];

/// Optimised blue noise rank table for lattice.
extern const std::uint32_t rankTable[size];

#else

/// Optimised blue noise key table for lattice.
constexpr std::uint32_t keyTable[] = {
inline constexpr std::uint32_t keyTable[] = {
#include "data/lattice/keys.txt"
};

/// Optimised blue noise rank table for lattice.
constexpr std::uint32_t rankTable[] = {
inline constexpr std::uint32_t rankTable[] = {
#include "data/lattice/ranks.txt"
};

#endif

} // namespace lattice

} // namespace bntables
Expand Down
2 changes: 1 addition & 1 deletion mkdocs/home.html
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ <h3>High Performance</h3>
<div class="oqmc-card">
<div class="oqmc-card__icon">🔧</div>
<h3>Easy Integration</h3>
<p>Header-only C++14 library with no dependencies. Compatible with VFX Reference Platform CY2018 and newer. Drop in and start sampling.</p>
<p>Header-only C++17 library with no dependencies. Compatible with VFX Reference Platform CY2021 and newer. Drop in and start sampling.</p>
</div>
</div>
</div>
Expand Down
51 changes: 32 additions & 19 deletions specs/cpp-17-migration/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,25 +189,38 @@ Update minimum tested compiler versions:

## Validation

The following must be carried out before the migration is accepted. They are
not yet done.

- **NVCC device-linkage**: The blue noise tables are consumed from
`OQMC_HOST_DEVICE` code (`bntables.h` `tableValue`), so they must be usable
on the device. The current internal-linkage `constexpr` arrays produce a
per-TU copy, which is what makes them trivially available in device code
today. `inline constexpr` changes this to a single external-linkage
definition, and NVCC's handling of inline-variable device-side definitions
has historically had caveats. Confirm on the minimum supported toolchain
(NVCC 11) that the tables compile, link, and produce correct results in
device code with no duplicate-definition or missing-symbol errors.

- **Compile-time trade-off**: A test/benchmark must be created to measure the
per-TU parse cost described under Migration risks on a representative
consumer (several translation units each including `bntables.h`). Compare
the C++14 binary variant, C++14 header-only, and the proposed C++17
`inline constexpr` builds for total compile time and final binary size, and
record the results here so the trade-off can be accepted explicitly.
- **NVCC device-linkage** (outstanding): The blue noise tables are consumed
from `OQMC_HOST_DEVICE` code (`bntables.h` `tableValue`), so they must be
usable on the device. The current internal-linkage `constexpr` arrays
produce a per-TU copy, which is what makes them trivially available in
device code today. `inline constexpr` changes this to a single
external-linkage definition, and NVCC's handling of inline-variable
device-side definitions has historically had caveats. Confirm on the
minimum supported toolchain (NVCC 11) that the tables compile, link, and
produce correct results in device code with no duplicate-definition or
missing-symbol errors. Note that the tables are only read from host code
in practice (each blue noise sampler copies them into its cache in
`initialiseCache`, which is host-only), but this must still be verified on
CUDA hardware before release.

- **Compile-time trade-off** (measured): A benchmark harness compiled 8
representative consumer translation units (each using all three blue noise
sampler variants, so all six tables are referenced) plus a main, with
`-O2`, best of 3 runs. Apple Silicon M-series, Clang 21.1.1:

| Configuration | Build time | Binary size |
| --------------------------- | ---------- | ----------- |
| C++14 header-only | 3.83 s | 12.73 MB |
| C++14 binary variant | 2.30 s | 1.64 MB |
| C++17 `inline constexpr` | 3.83 s | 1.64 MB |

The C++17 build matches the binary variant's final size exactly (the
linker deduplicates the single external-linkage definition) while staying
header-only, and all three builds produce bit-identical sample output. The
per-TU parse cost relative to the binary variant is ~170 ms per table
consuming TU on this machine, the same as the C++14 header-only build. In
real projects only TUs that include a blue noise sampler header pay this
cost. This trade-off is accepted.

## Candidate C++17 improvements

Expand Down
Loading
Loading