Java bindings for LP, MIP and QP - #1524
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesJava bindings
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mlubin
left a comment
mlubin
left a comment
There was a problem hiding this comment.
I saw the PR is closed, sending my comments as I had them already written up.
|
|
||
| extern "C" { | ||
|
|
||
| cuopt_int_t cuOptLoadParametersFromFile(cuOptSolverSettings settings, const char* path); |
There was a problem hiding this comment.
We should discuss merging these extensions into the C API.
There was a problem hiding this comment.
Mostly resolved, and split the way you asked. The Java-local shim is gone — cuopt_java_native_api.hpp no longer exists. The solver-statistics extensions became real C API calls in #1715 (CUOPT_SOLUTION_ATTR_* plus scalar getters), and the copy-out behaviour was #1706, fixed by #1734; both merged separately, so this PR's cpp/ diff against main is empty.
What remains is the problem-model side: cuopt_jni.cpp still includes pdlp/cuopt_c_internal.hpp for name setters, the quadratic getters and the problem category. That is tracked in #1703 and is the prerequisite for shipping the jar as a binary. @chris-maes asked for the quadratic getters to be a separate PR, which matches.
| @@ -0,0 +1,1367 @@ | |||
| /* | |||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
I'd recommend avoiding a test dependency on the python interface. The java interface should stand on its own.
There was a problem hiding this comment.
Agreed and done — PythonParityTest.java is removed. The suite has no Python dependency; the three remaining classes exercise the Java API on its own.
| @@ -0,0 +1,25 @@ | |||
| # cuOpt Java bindings (beta) | |||
|
|
|||
| This directory is an isolated, customer-specific beta module for the cuOpt | |||
There was a problem hiding this comment.
Is this how we want to ship it?
There was a problem hiding this comment.
We would want to follow cuvs and try to publish to maven https://mvnrepository.com/artifact/com.nvidia.cuvs/cuvs-java
|
Sorry that was an accident. Reopening. |
| @@ -0,0 +1,28 @@ | |||
| /home/cbrissette/cuopt/java/cuopt/src/main/java/com/nvidia/cuopt/linearprogramming/BatchSolve.java | |||
There was a problem hiding this comment.
Do we need these files ? may be we can delete all the run time files so developers can concentrate on main parts.
ramakrishnap-nv
left a comment
ramakrishnap-nv
left a comment
There was a problem hiding this comment.
Focused review on APIs and shipping (vs how cuvs ships Java).
APIs: the surface is broad and, pleasingly, closely in sync with the Python API — the algebraic Problem layer matches Python's (camelCase) modeling methods almost 1:1, and DataModel maps cleanly (snake_case→camelCase). A few parity gaps and Java-idiom nits are noted inline.
Shipping: the main blockers — don't commit target/, and wire the build into CI/release the way cuvs does (ci/build_java.sh/ci/test_java.sh, dependencies.yaml java key, workflow jobs, version marker, docs toctree).
Non-blocking review comments below.
| } | ||
|
|
||
| /** Return true for maximize and false for minimize, matching Python get_sense(). */ | ||
| public boolean getSense() { |
There was a problem hiding this comment.
Parity note (not a rename request): getSense() correctly matches the Python DataModel.get_sense() (bool, True=maximize) — good. Two parity gaps vs Python though: (1) Python puts set_initial_primal_solution/set_initial_dual_solution on DataModel, whereas here they're on SolverSettings; (2) Python DataModel also exposes getters this class seems to lack: get_quadratic_objective_{values,indices,offsets}, get_variable_names/get_row_names, get_objective_name/get_problem_name, get_ascii_row_types.
| resetSolvedValues(); | ||
| } | ||
|
|
||
| public Object getObjective() { |
There was a problem hiding this comment.
getObjective() returns Object — callers must downcast. Prefer a typed return (or overloads). Same for SolverSettings.getTypedParameter() / getMipCallbacks(). (The modeling API otherwise tracks the Python Problem layer 1:1 — nice.)
| public enum ProblemCategory { | ||
| LP(0), | ||
| MIP(1), | ||
| IP(2); |
There was a problem hiding this comment.
We should deprecate IP across the whole code base.
There was a problem hiding this comment.
Removed for Java.
There was a problem hiding this comment.
On the Java side this is now moot: ProblemCategory is gone entirely (5992826), so nothing here names IP. NativeProblem reads the engine's category and reduces it to a boolean, which folds the all-integer case into isMIP without surfacing it.
Deprecating IP across the whole code base is the broader change and is outside this PR — worth its own issue if you want it tracked, and I am happy to file one.
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
Thanks for adding the JAVA API. Let's make sure that capitalization is consistent before merging.
|
|
||
| Solver statistics are read as scalar solution attributes through | ||
| ``getIntAttribute`` and ``getFloatAttribute``, selected by a | ||
| ``CuOptConstants.CUOPT_SOLUTION_ATTR_*`` value. This mirrors the problem |
There was a problem hiding this comment.
Remove "This mirrors the C API". I don't think users need to be aware of another API.
There was a problem hiding this comment.
Removed in af5e69a. The sentence now just says a statistic added later becomes a new constant rather than a new method, without explaining where the constants come from.
| y.setMIPStart(2.0); | ||
|
|
||
| try (SolverSettings settings = new SolverSettings()) { | ||
| // The array follows the problem's variable-index order. |
There was a problem hiding this comment.
Maybe it's better to show a loop over a variable array in index order? So that the users knows how to get the "variable" index order.
There was a problem hiding this comment.
Done in af5e69a — the MIP start example now loops over getVariables() and assigns by variable.getIndex() instead of a hand-written {3.0, 2.0} literal, so the index order is demonstrated rather than asserted.
|
|
||
| settings.setMIPCallback( | ||
| (solutionBound, userData) -> | ||
| new MIPCallbackSolution(new double[] {3.0, 2.0}, 19.0), |
There was a problem hiding this comment.
I'm not sure what is happening here? Why is MIPCallbackSolution being created?
There was a problem hiding this comment.
Fair — the example showed the mechanics with no explanation of the purpose. Rewritten in af5e69a.
MIPSetSolutionCallback runs in the opposite direction to the incumbent callback: the solver asks your code for a solution to try, and MIPCallbackSolution is the payload carrying it back. It is for feeding in a solution found elsewhere — your own heuristic, or a result from a previous solve. Returning null declines and leaves the search untouched.
The docs now say that, and the example builds the array from getVariables() rather than showing a bare {3.0, 2.0} literal that hid the ordering requirement.
| ---------------- | ||
|
|
||
| A problem can be inspected through ``getConstraintMatrix`` and | ||
| ``getQuadraticObjectiveMatrix``. To examine an LP relaxation, build the problem |
| System.out.println(solution.getPrimalObjective()); | ||
| } | ||
|
|
||
| ``SolverSettings.addMIPStart`` takes a full variable-index-ordered array. It |
There was a problem hiding this comment.
I worry about the variable-index-order. I think we need to show users how to get this variable index order. Or show an alternative where they don't need to know the ordering of the variables.
There was a problem hiding this comment.
Fixed in af5e69a, and I think the worry was right — the docs asserted variable-index order without ever showing how to obtain it, so a reader could reasonably assume declaration order.
Two changes. The per-variable setMIPStart form now leads, since it sidesteps ordering entirely. Where a complete array is genuinely needed, the example builds it from getVariables() and indexes with variable.getIndex(), so the ordering comes from the problem rather than from the reader:
double[] values = new double[problem.getNumVariables()];
for (Variable variable : problem.getVariables()) {
values[variable.getIndex()] = startFor(variable);
}
settings.addMIPStart(values);
| } | ||
|
|
||
| The callback receives a defensive Java array containing the incumbent vector, | ||
| the incumbent objective, the current solution bound, and the user data object. |
There was a problem hiding this comment.
I think we need to provide a function that takes in the incumbent vector and a set of variables and returns the solution in the order. For examples
Variable[] vars = { z, y, x };
fromIncumbent(incumbent, vars) returns { 3.0, 2.0, 1.0}
assuming x = 1, y = 2, and z = 3 in the incumbent.
There was a problem hiding this comment.
Added as Problem.fromIncumbent in af5e69a, with your signature — Problem.fromIncumbent(incumbent, z, y, x) returns {3.0, 2.0, 1.0} for x=1, y=2, z=3. It is a static taking varargs, so the call site reads as your example did.
It throws IllegalArgumentException naming the variable if an index falls outside the array, rather than an opaque ArrayIndexOutOfBoundsException, which is the likely mistake if someone passes a variable from a different problem. Tested both the reordering and that failure.
chris-maes
left a comment
chris-maes
left a comment
There was a problem hiding this comment.
LGTM. Some minor comments about making the MIP callbacks easier to use. These could be addressed in a follow up PR.
Thanks for all the hard work. Excited to have a Java API.
The resolver properties added earlier did not prevent anything: java-build failed again on the first artifact it fetched, with a 429 from Maven Central and no retry. Those properties are transport-specific and are ignored unless the matching transport is the one in use, so they were a no-op here. cuopt_mvn now retries the invocation with exponential backoff, which does not depend on the transport. It retries only when the output looks like an artifact-resolution failure, so a compile or test failure is still returned immediately rather than being run four times. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
The MIP docs asserted that arrays are in variable-index order without showing how to obtain it, which left the reader to assume their declaration order matched. Both the MIP start and the set-solution callback now build the array from getVariables, so the ordering comes from the problem. For the other direction, fromIncumbent reads named variables out of an index-ordered array and returns them in the order asked for, so a callback can pick out the variables it cares about without indexing by hand. Also scope the convex reference to convex problems: the MIP-only Solution fields move to the MIP page, SolverMethod drops the UNSET sentinel, and the statistics section no longer refers users to the C API. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
|
/ok to test af5e69a |
| int iterations = solution.getIntAttribute( | ||
| CuOptConstants.CUOPT_SOLUTION_ATTR_LP_NUM_ITERATIONS); | ||
|
|
||
| Which attributes a solution carries depends on the class of problem that |
There was a problem hiding this comment.
Strike "Which" and replace with "This"
There was a problem hiding this comment.
Reworded in 5ebeb28. It now opens with the subject: "This depends on the class of problem that produced the solution."
|
|
||
| ``Problem.read`` loads a problem, choosing the parser from the file extension: | ||
| ``.mps``, ``.qps`` and ``.lp`` are recognised, along with their ``.gz``, | ||
| ``.bz2`` and ``.lz4`` variants. A boolean overload forces fixed-format MPS. |
There was a problem hiding this comment.
Boolean is typically capitalized
| ------------ | ||
|
|
||
| All solver settings are set through ``SolverSettings``. Use the overloaded | ||
| ``setSetting`` methods for string, integer, floating-point, and boolean |
There was a problem hiding this comment.
Fixed in 5ebeb28 — capitalized here too, since it names the type rather than the Java keyword.
| --------- | ||
|
|
||
| ``SolverSettings`` and ``Solution`` own native handles and implement | ||
| ``AutoCloseable``. Prefer try-with-resources. They also register a ``Cleaner`` |
There was a problem hiding this comment.
"Prefer try-with-resources." is not a complete sentence. I'm not sure what you're trying to say? Is it something like "These variables prefer try-with-resources"?
There was a problem hiding this comment.
You are right that it did not say anything. It was meant as an instruction to the reader, but read as a fragment. Rewritten in 5ebeb28:
"SolverSettings and Solution own native handles and implement AutoCloseable, so close them with try-with-resources. They also register a Cleaner fallback, but closing them deterministically keeps native memory pressure predictable."
| @@ -0,0 +1,85 @@ | |||
| # cuOpt Java bindings | |||
There was a problem hiding this comment.
Bindings should be capitalized since it's a heading
| The standalone native project links to `${CUOPT_PREFIX}/lib/libcuopt.so`. No | ||
| Java-specific symbol or source file is required by the main cuOpt build. | ||
|
|
||
| ## JNI symbol check |
There was a problem hiding this comment.
Capitalize Symbol Check
| bash scripts/check_jni_symbols.sh | ||
| ``` | ||
|
|
||
| ## Generated constants |
| SPDX-License-Identifier: Apache-2.0 | ||
| --> | ||
|
|
||
| # cuOpt Java binding tests |
There was a problem hiding this comment.
Capitalize Binding Tests
There was a problem hiding this comment.
Fixed in 5ebeb28 — "cuOpt Java Binding Tests".
|
|
||
| The suite has no dependency on the cuOpt Python interface. | ||
|
|
||
| ## How to run |
| equality and ranged constraints, mixed bounds, mixed integer/continuous | ||
| variables, metadata, and infeasibility. | ||
|
|
||
| ## Prerequisite behavior |
There was a problem hiding this comment.
Fixed in 5ebeb28 — "Prerequisite Behavior". That completes the headings in both files; "Building" and "Coverage" were already title case.
cwilkinson76
left a comment
cwilkinson76
left a comment
There was a problem hiding this comment.
A few minor edits needed
Title-case the README and TESTS headings, capitalize Boolean as the type name, and replace the sentence fragment describing AutoCloseable with one that says what to do. Reword the solution-attribute sentence to open with its subject. Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
|
/ok to test 5ebeb28 |
|
@cwilkinson76 May I get another round of review on this ? |
|
/merge |
6 participants
Description
Java bindings for LP, MIP and QP, built as hand-written JNI over the public C API, with build and CI integration. Contributes to #1535 and #860.
The API is deliberately small. Following review, it covers building a problem, solving it, and reading the result — nothing beyond that. Anything further can be added later with a case for it.
API
Problembuilds and solves;VariableandConstraintcarry the model and the values from the last solve;SolverSettingsconfigures the solver;Solutionreports the outcome.Solution values are read from the model —
Variable.getValue,Variable.getReducedCost,Constraint.getDualValue,Constraint.getSlack— rather than as bulk arrays.Solver statistics are read as scalar solution attributes, keyed by a
CuOptConstants.CUOPT_SOLUTION_ATTR_*value:This mirrors the problem attribute accessors. Because the selectors are generated into
CuOptConstantsfromconstants.h, a statistic added later is a new constant rather than new Java. A selector that does not apply to the solution, or that does not have the requested value type, raisesCuOptException.Build integration
javais an opt-in target of the top-levelbuild.sh:It prefers the
cpp/buildtree so it works without--install, and falls back to the conda prefix, which is what CI uses.ci/build_java.shcallsbuild.shrather than duplicating the invocation.The rmm and raft headers must be the ones
libcuoptwas compiled against — rmm carries its version in an inline namespace, so a mismatched copy links cleanly and then fails atdlopen.build.shpasses the right include and library paths when it targets a build tree.CI
pr.yamljava-buildgpu-l4-latest-1ci/test_java.shbuild.yamljava-buildcpu4ci/build_java.shtest.yamlconda-java-testsgpu-l4-latest-1ci/test_java.shAll three are in their workflow's aggregator job, so a Java failure fails the PR. The PR job is gated on the
test_javachanged-files group (java/**,ci/build_java.sh,ci/test_java.sh) and ontest_cpp, so a C++ change that could break the bindings still exercises them.target/is uploaded as thecuopt-javaartifact.The
javadependency file-key pullslibcuoptplus thelibraft-headers,librmmandrapids-loggerheaders thatlibcuopt's public headers include transitively.ci/release/update-version.shbumps the POM through a sentinel comment; the artifact version drops the zero-padded RAPIDS patch field, since Maven has no notion of it.JNI symbol check
The bindings are hand-written, so a
static nativedeclaration and its entry point can drift apart. JNI resolves lazily, so the library still loads and the failure appears only when something calls the method.scripts/check_jni_symbols.shdiffs the prototypesjavac -hderives from the Java sources against the symbols the built library exports, and fails on a mismatch either way. It reads the built artifact rather than parsing source, so the macro-generated entry points need no special casing.build_native.shruns it after every native build, which covers./build.sh javaand both CI jobs. It takes about a second.Packaging
Version
26.10.0. Sources and javadoc jars are attached, and the POM carries theurl,licenses,scmanddevelopersmetadata Maven Central requires. Signing and upload are not wired up; that is tracked by the "Publishing & support" item in #1535.Known gap
cuopt_jni.cppstill includespdlp/cuopt_c_internal.hppfor parts of the problem path the C API does not cover — setting names, the quadratic objective and quadratic constraint getters, and the problem category. That coupleslibcuopt_jni.soto a specificlibcuoptbuild rather than to a stable ABI, which matters before this ships as a binary artifact. Tracked in #1703. The related copy-out behaviour was #1706, fixed by #1734 and now inmain.Checklist