add checks that dense A is constructed

[AlexandreAmice]

This change is 

[AlexandreAmice]

Addressing #20516. cc @hongkai-dai, @jwnimmer-tri, @EricCousineau-TRI.

Quickly adding the test that @jwnimmer-tri suggested shows that the dense A matrix is being constructed, suggesting that the dense constructor is indeed called. I will try to investigate further soon.

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on @AlexandreAmice)

[EricCousineau-TRI]

I can make your example fail the is_dense check by applying this patch:

$ git diff -U1
diff --git a/bindings/pydrake/solvers/BUILD.bazel b/bindings/pydrake/solvers/BUILD.bazel
index a306173a4f..f45c53f805 100644
--- a/bindings/pydrake/solvers/BUILD.bazel
+++ b/bindings/pydrake/solvers/BUILD.bazel
@@ -232,3 +232,3 @@ drake_py_unittest(
diff --git a/bindings/pydrake/solvers/BUILD.bazel b/bindings/pydrake/solvers/BUILD.bazel
index a306173a4f..f45c53f805 100644
--- a/bindings/pydrake/solvers/BUILD.bazel
+++ b/bindings/pydrake/solvers/BUILD.bazel
@@ -232,3 +232,3 @@ drake_py_unittest(
         "//bindings/pydrake/common/test_utilities",
-        "//bindings/pydrake/common/test_utilities:scipy_stub_py",
+        # "//bindings/pydrake/common/test_utilities:scipy_stub_py",
         "//bindings/pydrake/math",
diff --git a/bindings/pydrake/solvers/test/mathematicalprogram_test.py b/bindings/pydrake/solvers/test/mathematicalprogram_test.py
index 4bad1785d8..d8fa7f118b 100644
--- a/bindings/pydrake/solvers/test/mathematicalprogram_test.py
+++ b/bindings/pydrake/solvers/test/mathematicalprogram_test.py
@@ -804,5 +804,5 @@ class TestMathematicalProgram(unittest.TestCase):

-        A_sparse = scipy.sparse.csc_matrix(
-            (np.array([2, 1, 3]), np.array([0, 1, 0]),
-             np.array([0, 2, 2, 3])), shape=(2, 2))
+        # A_sparse = scipy.sparse.csc_matrix(
+        #     (np.array([2, 1, 3]), np.array([0, 1, 0]),
+        #      np.array([0, 2, 2, 3])), shape=(2, 2))

Then running:

bazel run //bindings/pydrake/solvers:py/mathematicalprogram_test -- TestMathematicalProgram.test_linear_constraints

In this case, the test should be run in a case w/o the scipy_stub

[EricCousineau-TRI]

Here's a smaller patch that shows the failure similar to what I posted:
929d28e

I think the issue is that the scipy_stub seems to have different behavior than the scipy that I have on my system.
Mine is installed via apt install python3-scipy (Ubuntu 22.04 (Jammy)), version 1.8.0-1exp2ubuntu1

FYI @jwnimmer-tri

[jwnimmer-tri]

I think the issue is that the scipy_stub seems to have different behavior than the scipy that I have on my system.

Without having tried it, I'd guess that the reason for the difference is this pybind11 conversion code for Eigen::Sparse:

        if (!type::handle_of(obj).is(matrix_type)) {
            try {
                obj = matrix_type(obj);
            } catch (const error_already_set &) {
                return false;
            }
        }

In the stub, the csc_matrix is not constructible from a 1-argument dense matrix -- our stub requires the shape argument. In real scipy, I assume it is constructible.

So we could fix that in the stub, but what's not clear to me is why when we have the overloads bound in this order ...

      .def("AddLinearConstraint",
          static_cast<Binding<LinearConstraint> (MathematicalProgram::*)(
              const Eigen::Ref<const Eigen::MatrixXd>&,
              const Eigen::Ref<const Eigen::VectorXd>&,
              const Eigen::Ref<const Eigen::VectorXd>&,
              const Eigen::Ref<const VectorXDecisionVariable>&)>(
              &MathematicalProgram::AddLinearConstraint),
          py::arg("A"), py::arg("lb"), py::arg("ub"), py::arg("vars"),
          doc.MathematicalProgram.AddLinearConstraint.doc_4args_A_lb_ub_dense)
      .def("AddLinearConstraint",
          static_cast<Binding<LinearConstraint> (MathematicalProgram::*)(
              const Eigen::Ref<const Eigen::RowVectorXd>&, double, double,
              const Eigen::Ref<const VectorXDecisionVariable>&)>(
              &MathematicalProgram::AddLinearConstraint),
          py::arg("a"), py::arg("lb"), py::arg("ub"), py::arg("vars"),
          doc.MathematicalProgram.AddLinearConstraint.doc_4args_a_lb_ub_vars)

... the second one is even being run at all. The first one should have matched?!

[jwnimmer-tri]

Oh, duh. The problem is that this pybind11 code for Eigen::Sparse does not obey the bool convert second argument to the loader:

        if (!type::handle_of(obj).is(matrix_type)) {
            try {
                obj = matrix_type(obj);

[jwnimmer-tri]

I pushed a commit here that improves the test stub so that it can demonstrate the failure mode. The unit tests (correctly) show "is not dense" failures now.

I've filed the pybind11 upgrade with the fix at #20524. It fixes the test errors that show up with the new stub. My suggestion is that we work to merge that upgrade, and then return here to land the additional testing in a second pass.

[EricCousineau-TRI]

Pushed changes using your patch (confirmed failure - thanks!), and fixed linting issues.

+@EricCousineau-TRI feature
+@jwnimmer-tri Would you be up for platform?

Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

[jwnimmer-tri]

platform.

Thanks for the help @AlexandreAmice and @EricCousineau-TRI.

Reviewed 1 of 1 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

bindings/pydrake/common/test_utilities/scipy_stub/scipy/sparse/__init__.py line 27 at r3 (raw file):

            rows, cols = dense.shape
            self.shape = (rows, cols)
            self.data = []

BTW @EricCousineau-TRI did you read over this new logic thoroughly? I'm generally pretty hamfisted with matrices, so please be sure you gave it a good look.

[jwnimmer-tri]

Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @AlexandreAmice)

---

bindings/pydrake/common/test_utilities/scipy_stub/scipy/sparse/__init__.py line 27 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

BTW @EricCousineau-TRI did you read over this new logic thoroughly? I'm generally pretty hamfisted with matrices, so please be sure you gave it a good look.

Meh, perfect need not be the enemy of the good. If the test stub is wrong, we can circle back and fix it later.

[EricCousineau-TRI]

Sorry, was gonna ask if we could add a test. PR'd test + bugfix in #20541