Add summary statistics to ProductMeasure - #608
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #608 +/- ##
===========================================
+ Coverage 88.65% 88.70% +0.04%
===========================================
Files 104 104
Lines 8508 8561 +53
===========================================
+ Hits 7543 7594 +51
- Misses 965 967 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds combined summary statistics to ProductMeasure.
Changes:
- Concatenates marginal means, variances, and standard deviations.
- Constructs block-diagonal covariance matrices.
- Adds coverage for scalar, multidimensional, ordering, and unavailable-statistic cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
qmcpy/true_measure/product_measure.py |
Implements combined statistics. |
test/test_product_measure.py |
Tests statistic aggregation and errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if sparse.issparse(block): | ||
| block = block.toarray() |
There was a problem hiding this comment.
Fixed in commit Fix ProductMeasure.covariance to preserve sparse blocks. The covariance property now:
- Skips
toarray()on sparse blocks, preserving their shape for validation - Uses
sparse.block_diag(blocks, format="dia")and sets.data.setflags(write=False)when all blocks are sparse - Falls back to the dense path (with
toarray()only at that point) when blocks are mixed sparse/dense
Co-authored-by: fjhickernell <817530+fjhickernell@users.noreply.github.com>
fjhickernell
left a comment
There was a problem hiding this comment.
tried it with the AreWeThereYet notebook for MATH 565 and it worked like a charm.
|
@Laasya-73, running by copilot once more but it looks ready for you to squash and merge. |
| np.testing.assert_allclose( | ||
| tm.standard_deviation, [np.sqrt(4.0 / 3.0), np.sqrt(3.0)] | ||
| ) | ||
| np.testing.assert_allclose(tm.covariance, np.diag([4.0 / 3.0, 3.0])) |
There was a problem hiding this comment.
Fixed in the latest commit. The covariance result is now converted to a dense array via toarray() when sparse before passing to assert_allclose.
| for statistic in ("mean", "variance", "standard_deviation", "covariance"): | ||
| assert not getattr(tm, statistic).flags.writeable |
There was a problem hiding this comment.
Fixed in the latest commit. The immutability loop now checks value.data.flags for sparse matrices and value.flags for dense arrays.
Co-authored-by: fjhickernell <817530+fjhickernell@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
qmcpy/true_measure/product_measure.py:267
- Return a protected view here, not the owned array itself. An owned array made read-only with
setflags(write=False)can be made writable again by callingresult.setflags(write=True), whereas the established statistics contract explicitly checks that this raises (test/test_true_measures.py:221-225). The mixed sparse/dense covariance path therefore exposes a mutable result unlike the other dense statistics.
return self._read_only_array(covariance)
|
A dense block densifies an otherwise sparse covariance: |
This PR adds combined summary statistics to
ProductMeasure.ProductMeasurenow derives statistics for its independent marginal blocks by:This allows users to access statistics for the complete product distribution directly instead of calculating them separately for each marginal.
Changes
ProductMeasure.