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
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@

#include <cuda/std/span>

namespace cuopt::cython {
class lp_solve_session_t;
}

namespace cuopt::linear_programming {

// Forward declare solver_settings_t for friend class
Expand Down Expand Up @@ -326,6 +330,10 @@ class pdlp_solver_settings_t {
// Used to force batch PDLP to solve a subbatch of the problems at a time
// The 0 default value will make the solver use its heuristic to determine the subbatch size
i_t fixed_batch_size{0};
/** When true, first GPU barrier/QCQP solve returns an ``lp_solve_session_t`` capsule. */
bool session_enabled{false};
/** Non-owning session pointer set by ``call_solve`` for barrier symbolic reuse. */
cuopt::cython::lp_solve_session_t* lp_solve_session{nullptr};
Comment on lines +334 to +336

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we separate the session pointer from solver_settings_t struct? It is set to nullptr in every call of call_solve in cython_solve.cu. Parameters in solver_settings_t shouldn't be changed within the algorithm call.


private:
/** Initial primal solution */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <cuopt/linear_programming/optimization_problem_solution_interface.hpp>
#include <cuopt/linear_programming/solver_settings.hpp>
#include <cuopt/linear_programming/utilities/cython_types.hpp>
#include <cuopt/linear_programming/utilities/lp_solve_session.hpp>

#include <cuopt/linear_programming/io/data_model_view.hpp>
#include <memory>
Expand Down Expand Up @@ -55,7 +56,8 @@ std::unique_ptr<solver_ret_t> call_solve(
cuopt::linear_programming::io::data_model_view_t<int, double>*,
linear_programming::solver_settings_t<int, double>*,
unsigned int flags = cudaStreamNonBlocking,
bool is_batch_mode = false);
bool is_batch_mode = false,
lp_solve_session_t* session_in = nullptr);

std::pair<std::vector<std::unique_ptr<solver_ret_t>>, double> solve_batch_remote(
std::vector<cuopt::linear_programming::io::data_model_view_t<int, double>*>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <cuopt/linear_programming/mip/solver_solution.hpp>
#include <cuopt/linear_programming/pdlp/solver_solution.hpp>
#include <cuopt/linear_programming/utilities/internals.hpp>
#include <cuopt/linear_programming/utilities/lp_solve_session.hpp>

#include <rmm/device_buffer.hpp>

Expand Down Expand Up @@ -85,6 +86,9 @@ struct linear_programming_ret_t {
double solve_time_{};
linear_programming::method_t solved_by_{};

/** GPU barrier session (stream + handle + symbolic cache); moved to Python capsule when set. */
std::unique_ptr<lp_solve_session_t> lp_solve_session;

bool is_gpu() const { return std::holds_alternative<gpu_solutions_t>(solutions_); }
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/* clang-format off */
/*
* SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/
/* clang-format on */

#pragma once

#include <memory>

#include <raft/core/handle.hpp>
#include <rmm/cuda_stream.hpp>

namespace cuopt::linear_programming::dual_simplex {
template <typename i_t, typename f_t>
class iteration_data_t;
template <typename i_t, typename f_t>
struct barrier_symbolic_cache_t;

template <typename i_t, typename f_t>
void barrier_store_symbolic_cache_from_iteration_data(iteration_data_t<i_t, f_t>& data,
barrier_symbolic_cache_t<i_t, f_t>& cache);
} // namespace cuopt::linear_programming::dual_simplex

namespace cuopt::cython {

/**
* @brief Lean GPU solve session: owns RAFT handle + stream and optional barrier symbolic cache.
*
* Created on first solve when session_enabled; reused on subsequent solves with the same capsule.
* Per-solve state (optimization_problem_t, presolve, barrier_lp) remains stack-local.
*/
class lp_solve_session_t {
public:
static std::unique_ptr<lp_solve_session_t> create(unsigned stream_flags);

lp_solve_session_t(lp_solve_session_t&&) noexcept;
lp_solve_session_t& operator=(lp_solve_session_t&&) noexcept;
~lp_solve_session_t();

[[nodiscard]] raft::handle_t* handle_ptr();
[[nodiscard]] raft::handle_t const* handle_ptr() const;
[[nodiscard]] rmm::cuda_stream_view stream_view() const;

/**
* @brief Returns cached symbolic state when valid and @p handle matches the stored handle.
*/
[[nodiscard]] linear_programming::dual_simplex::barrier_symbolic_cache_t<int, double>*
symbolic_cache_for_reuse(raft::handle_t const* handle);

void clear_symbolic_cache();

void store_symbolic_cache(
linear_programming::dual_simplex::iteration_data_t<int, double>& data);

private:
lp_solve_session_t(std::unique_ptr<rmm::cuda_stream> stream,
std::unique_ptr<raft::handle_t> handle);

struct impl;
std::unique_ptr<impl> impl_;
};
Comment on lines +28 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files cpp/include/cuopt/linear_programming/utilities/lp_solve_session.hpp
printf '\n--- file outline ---\n'
ast-grep outline cpp/include/cuopt/linear_programming/utilities/lp_solve_session.hpp --view expanded || true
printf '\n--- file excerpt ---\n'
cat -n cpp/include/cuopt/linear_programming/utilities/lp_solve_session.hpp | sed -n '1,220p'
printf '\n--- search for lp_solve_session_t references ---\n'
rg -n "lp_solve_session_t|thread-safety|thread safety|concurrent|serialize access|session_enabled|symbolic_cache_for_reuse|store_symbolic_cache|clear_symbolic_cache" cpp/include cpp/src -g '!**/build/**' || true

Repository: NVIDIA/cuopt

Length of output: 29589


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n--- lp_solve_session implementation ---\n'
cat -n cpp/src/pdlp/utilities/lp_solve_session.cu | sed -n '1,220p'

printf '\n--- cython solve usage around session lifecycle ---\n'
cat -n cpp/src/pdlp/utilities/cython_solve.cu | sed -n '120,260p'

printf '\n--- public header with session field docs ---\n'
cat -n cpp/include/cuopt/linear_programming/pdlp/solver_settings.hpp | sed -n '320,350p'

Repository: NVIDIA/cuopt

Length of output: 13671


Add method docs and spell out the threading contract. The public methods here (create, handle_ptr, stream_view, clear_symbolic_cache, store_symbolic_cache) still need Doxygen comments, and the session should explicitly say whether callers can reuse one instance from multiple threads. The cache mutators are unsynchronized, so the intended access pattern should be stated plainly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/include/cuopt/linear_programming/utilities/lp_solve_session.hpp` around
lines 28 - 63, The public API on lp_solve_session_t is missing Doxygen and an
explicit threading contract. Add comments for create, handle_ptr, stream_view,
clear_symbolic_cache, and store_symbolic_cache describing ownership, return
values, and cache behavior. Also state clearly that lp_solve_session_t is not
thread-safe for concurrent use, and that the unsynchronized cache mutators and
reuse helpers must only be called from a single thread or externally serialized
context.

Source: Path instructions


} // namespace cuopt::cython
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/* clang-format off */
/*
* SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/
/* clang-format on */

#pragma once

#include <utilities/logger.hpp>

#include <array>
#include <chrono>
#include <cstdarg>
#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <string>

namespace cuopt::linear_programming::cache_profile {

enum class cache_id : int {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it only for temporary code verification?

C01 = 0, // raft::handle_t + stream
C02, // cuBLAS / cuSparse warmup (init_handler)
C03, // problem fingerprint (structural hash)
C04, // augmented vs ADAT choice
C05, // ADAT / augmented sparsity pattern
C06, // cuDSS handle + config
C07, // cuDSS symbolic factorization
C08, // dense-column / SOC layout metadata
C09, // device buffer allocation (iteration_data setup)
COUNT
};

inline constexpr int num_cache_ids = static_cast<int>(cache_id::COUNT);

inline const char* cache_id_label(cache_id id)
{
switch (id) {
case cache_id::C01: return "C01 raft handle+stream";
case cache_id::C02: return "C02 cuBLAS/cuSparse warmup";
case cache_id::C03: return "C03 problem fingerprint";
case cache_id::C04: return "C04 augmented vs ADAT choice";
case cache_id::C05: return "C05 KKT sparsity pattern";
case cache_id::C06: return "C06 cuDSS handle+config";
case cache_id::C07: return "C07 cuDSS symbolic factorization";
case cache_id::C08: return "C08 dense-column/SOC layout";
case cache_id::C09: return "C09 device buffer allocation";
default: return "C?? unknown";
}
}

class profiler_t {
public:
static profiler_t& instance()
{
static profiler_t prof;
return prof;
}

bool enabled() const { return enabled_; }

void reset()
{
times_.fill(0.0);
}

void add(cache_id id, double seconds)
{
if (!enabled_) { return; }
times_[static_cast<int>(id)] += seconds;
}

double get(cache_id id) const { return times_[static_cast<int>(id)]; }

double total_measured() const
{
double sum = 0.0;
for (double t : times_) {
sum += t;
}
return sum;
}

void log_summary() const
{
if (!enabled_) { return; }
auto emit = [](const char* fmt, ...) {
va_list args;
va_start(args, fmt);
char buf[512];
vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);
CUOPT_LOG_INFO("%s", buf);
fprintf(stderr, "%s\n", buf);
};
emit("=== Solver cache profile (ms) ===");
for (int i = 0; i < num_cache_ids; ++i) {
const double ms = times_[i] * 1000.0;
emit("Cache profile: %s %.3f", cache_id_label(static_cast<cache_id>(i)), ms);
}
emit("Cache profile: TOTAL measured %.3f", total_measured() * 1000.0);
emit("=== End solver cache profile ===");
}

private:
profiler_t()
{
const char* env = std::getenv("CUOPT_CACHE_PROFILE");
enabled_ = env != nullptr && env[0] != '\0' && std::strcmp(env, "0") != 0;
}

bool enabled_{false};
std::array<double, num_cache_ids> times_{};
};
Comment on lines +54 to +116

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the profiler and its call sites
git ls-files | rg 'solver_cache_profiler|cython_solve|solver_settings|solve\.cu|Concurrent'

# Show the profiler header with line numbers
sed -n '1,220p' cpp/include/cuopt/linear_programming/utilities/solver_cache_profiler.hpp

# Find all references to cache_profile::reset() and profiler_t::instance()
rg -n "cache_profile::reset\(|profiler_t::instance\(|CUOPT_CACHE_PROFILE|method_t::Concurrent|Concurrent" cpp include python -S

# Inspect the likely solve entry point and concurrent execution references
sed -n '1,260p' cpp/src/linear_programming/cython_solve.cu
sed -n '1,260p' cpp/include/cuopt/linear_programming/solver_settings.hpp
sed -n '1,260p' cpp/src/linear_programming/pdlp/solve.cu

Repository: NVIDIA/cuopt

Length of output: 16099


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the solve entry point and profiler reset site
sed -n '90,150p' cpp/src/pdlp/utilities/cython_solve.cu

# Inspect the method_t documentation and default concurrent behavior
sed -n '50,90p' cpp/include/cuopt/linear_programming/pdlp/solver_settings.hpp
sed -n '300,330p' cpp/include/cuopt/linear_programming/pdlp/solver_settings.hpp

# Inspect the main PDLP solve path around Concurrent handling
sed -n '1810,1865p' cpp/src/pdlp/solve.cu

# Check for any synchronization around the profiler or cache_profile namespace
rg -n "mutex|lock_guard|atomic|cache_profile::|CUOPT_CACHE_PROFILE_SCOPE" cpp/src/pdlp cpp/src/barrier cpp/src/dual_simplex cpp/include/cuopt/linear_programming -S

Repository: NVIDIA/cuopt

Length of output: 12850


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the concurrent solve implementation
sed -n '1,220p' cpp/src/pdlp/solve.cu | sed -n '1,220p'
rg -n "run_concurrent|std::thread|std::async|omp|launch|concurrent_halt|Barrier|DualSimplex" cpp/src/pdlp/solve.cu cpp/src/barrier cpp/src/dual_simplex -S

# Inspect the cache-profile usage sites in the concurrent backends
sed -n '1,120p' cpp/src/barrier/barrier.cu
sed -n '1820,1975p' cpp/src/pdlp/solve.cu

Repository: NVIDIA/cuopt

Length of output: 50369


Guard the process-wide profiler state. profiler_t::reset() and add() mutate a singleton times_ array with no synchronization. Since run_concurrent() schedules work on OMP tasks, overlapping timers and concurrent solves can race with reset(), corrupting the reported cache profile. A mutex here would be small; otherwise document that CUOPT_CACHE_PROFILE is single-threaded only.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/include/cuopt/linear_programming/utilities/solver_cache_profiler.hpp`
around lines 54 - 116, Guard the singleton profiler state in profiler_t:
reset(), add(), and log_summary() all read/write the shared times_ array without
synchronization, so concurrent OMP task execution can race and corrupt cache
profiling. Add a mutex or other thread-safe protection around the mutable state
in profiler_t::instance(), profiler_t::reset(), profiler_t::add(), and any
readout in profiler_t::log_summary(), or explicitly restrict CUOPT_CACHE_PROFILE
to single-threaded use if locking is not desired.

Source: Path instructions


inline bool enabled() { return profiler_t::instance().enabled(); }

inline void reset() { profiler_t::instance().reset(); }

inline void add(cache_id id, double seconds) { profiler_t::instance().add(id, seconds); }

inline void log_summary() { profiler_t::instance().log_summary(); }

class scoped_timer_t {
public:
explicit scoped_timer_t(cache_id id) : id_(id), start_(clock_::now()), active_(enabled()) {}

~scoped_timer_t()
{
if (!active_) { return; }
const double elapsed =
std::chrono::duration<double>(clock_::now() - start_).count();
add(id_, elapsed);
}

private:
using clock_ = std::chrono::steady_clock;
cache_id id_;
clock_::time_point start_;
bool active_;
};

} // namespace cuopt::linear_programming::cache_profile

#define CUOPT_CACHE_PROFILE_SCOPE(id) \
::cuopt::linear_programming::cache_profile::scoped_timer_t CUOPT_CACHE_PROFILE_CONCAT( \
_cuopt_cache_scope_, __LINE__)(id)

#define CUOPT_CACHE_PROFILE_CONCAT(a, b) CUOPT_CACHE_PROFILE_CONCAT_IMPL(a, b)
#define CUOPT_CACHE_PROFILE_CONCAT_IMPL(a, b) a##b
1 change: 1 addition & 0 deletions cpp/src/barrier/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
set(BARRIER_SRC_FILES
${CMAKE_CURRENT_SOURCE_DIR}/cusparse_view.cu
${CMAKE_CURRENT_SOURCE_DIR}/barrier.cu
${CMAKE_CURRENT_SOURCE_DIR}/barrier_factorization_sparsity_hash.cu
${CMAKE_CURRENT_SOURCE_DIR}/device_sparse_matrix.cu
${CMAKE_CURRENT_SOURCE_DIR}/pinned_host_allocator.cu
)
Expand Down
Loading