Support domain inclusion in chained TrueMeasure transformations - #609
Support domain inclusion in chained TrueMeasure transformations#609Laasya-73 wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #609 +/- ##
===========================================
+ Coverage 88.65% 88.71% +0.05%
===========================================
Files 104 104
Lines 8508 8530 +22
===========================================
+ Hits 7543 7567 +24
+ Misses 965 963 -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
Generalizes chained TrueMeasure compatibility from exact range/domain equality to interval containment.
Changes:
- Adds broadcast-aware range containment checks and revised errors.
- Adds unit tests for valid and invalid chains.
- Adds a detailed demonstration notebook.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
qmcpy/true_measure/abstract_true_measure.py |
Implements containment-based compatibility. |
test/test_true_measures.py |
Tests containment and chaining behavior. |
demos/true_measure_domain_inclusion.ipynb |
Demonstrates the new semantics and mathematical context. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| lower_bounds_valid = np.all(domain[:, 0] <= transform_range[:, 0]) | ||
| upper_bounds_valid = np.all(transform_range[:, 1] <= domain[:, 1]) | ||
| return bool(lower_bounds_valid and upper_bounds_valid) |
| "import matplotlib.pyplot as plt\n", | ||
| "import numpy as np\n", | ||
| "import pandas as pd\n", | ||
| "from matplotlib.patches import Rectangle\n", | ||
| "from scipy.stats import logistic, norm\n", | ||
| "\n", | ||
| "from qmcpy import DigitalNetB2, Kumaraswamy, Uniform\n", | ||
| "from qmcpy.true_measure.abstract_true_measure import AbstractTrueMeasure\n", | ||
| "from qmcpy.util import ParameterError\n", |
|
@Laasya-73 you should add some examples in the demo notebook of integration with the chained measures. |
Sure, I will add and update the PR. |
There was a problem hiding this comment.
- Could you add the notebook to
mkdocs.yml? Thank you. - Validate input to
_range_in_domain:
>>> AbstractTrueMeasure._range_in_domain(np.empty((0, 2)),[[0, 1]])
True
>>> AbstractTrueMeasure._range_in_domain([["a", "z"]], [["a", "z"]])
True
-
In the notebook,
from qmcpy import DigitalNetB2, Kumaraswamy, Uniform from qmcpy.true_measure.abstract_true_measure import AbstractTrueMeasurecan be simplified tofrom qmcpy import AbstractTrueMeasure, DigitalNetB2, Kumaraswamy, Uniform -
In a command line window, you could run "make format" to beautify the files. Give it a try and see if it helps.
-
For the notebook sub-section title, "### One compatibility visual", do you want to make it "D."? Sections 3 and 5 have sub-section labels. Would you like to make them consistent?
|
Please request for re-review frojm us whenever the changes are ready. |
|
Thanks! The changes are now addressed and pushed. I’ve re-requested your review. |
There was a problem hiding this comment.
Thank you for the continuous enhancements.
Could you please review the following examples for me? I want to ensure that I have not missed anything. Please add fast unit tests whenever applicable.
- Issues with range, mean, and variance, as shown. in the following examples:
(a)
>>> from qmcpy import *
>>> inner = Uniform(DigitalNetB2(1), 0.25, 0.75)
>>> outer = Uniform(inner, 0.25, 0.75)
>>> samples = outer.gen_samples(2**14)
>>> declared_range = outer.range.ravel()
>>> composite_range = 0.25 + 0.5 * inner.range.ravel()
>>> declared_variance = outer.variance
>>> composite_variance = 0.25**2 / 12
>>> sample_variance = samples.var()
>>> print(f" {declared_range=} vs. {composite_range=} Cf. observed min/max: [{samples.min():.5f}, {samples.max():.5f}]")
declared_range=array([0.25, 0.75]) vs. composite_range=array([0.375, 0.625]) Cf. observed min/max: [0.37501, 0.62500]
>>> print(f" {declared_variance=:.6f} vs. actual/sample variance: {composite_variance:.6f} / {sample_variance:.6f}")
declared_variance=0.020833 vs. actual/sample variance: 0.005208 / 0.005208
(b)
inner = Uniform(DigitalNetB2(1, seed=7), 0.25, 0.75)
outer = Kumaraswamy(inner)
samples = outer.gen_samples(2**15)
declared_mean = float(np.ravel(outer.mean)[0]) # 0.53333
sample_mean = float(samples.mean()) # .53971
(c)
>>> import numpy as np
>>> for outer in [Kumaraswamy(Uniform(DigitalNetB2(1, seed=7), 0.25, 0.75)),
... Gaussian(Uniform(DigitalNetB2(1, seed=7), 0.25, 0.75))]:
... s = outer.gen_samples(2**15)
... dv, sv = float(np.ravel(outer.variance)[0]), s.var()
... print(f"{type(outer).__name__:12s} declared var {dv:.6f} vs sample {sv:.6f} ({abs(dv-sv)/dv:.0%} off)")
Kumaraswamy declared var 0.048889 vs sample 0.009355 (81% off)
Gaussian declared var 1.000000 vs sample 0.142652 (86% off)
(d)
inner = BernoulliCont(DigitalNetB2(1, seed=7), lam=0.9)
outer = Kumaraswamy(inner, a=2.0, b=2.0)
samples = outer.gen_samples(2**16)
declared_variance = float(np.ravel(outer.variance)[0]) # 0.0489
sample_variance = samples.var() # 0.0384
-infis returned in the following example. Please look into it:
>>> Gaussian(Uniform(DigitalNetB2(1, randomize="FALSE"), 0.0, 0.75)).gen_samples(2)
array([[ -inf],
[-0.31863936]])
_weight()returns non-zero weights outside domain:
dd = DigitalNetB2(1, seed=7)
uniform_weights = Uniform(dd, 0.25, 0.75)._weight(np.array([[0.1], [0.3], [0.5], [0.9]])).ravel() # [2., 2., 2., 2.]
kumaraswamy_weights = Kumaraswamy(dd, a=2, b=2)._weight(np.array([[-0.5], [0.5], [1.5]])).ravel() # [-1.5, 1.5, -7.5]
|
Thank you for these examples. I reviewed all four composition cases and added fast unit tests covering For 1, I found that the discrepancy comes from the existing recursive composition behavior: samples apply each transform in sequence, while the outer I also fixed the Gaussian endpoint issue in 2 and the out-of-support |
There was a problem hiding this comment.
Thank you for the quick turn around.
-
I have pulled your code and rerun examples in comment #1 above but the issues seem to still exist. Could you check them again?
-
We still have
-infwith the following three instances:
JohnsonsSU(DigitalNetB2(1, randomize="FALSE")).gen_samples(2)
BrownianMotion(DigitalNetB2(2, randomize="FALSE"),
decomp_type="BROWNIANBRIDGE").gen_samples(2)
SciPyWrapper(DigitalNetB2(1, randomize="FALSE"), _st.norm()).gen_samples(2)
- An additional exception that I have found today is as follows:
m = Kumaraswamy(Uniform(DigitalNetB2(1), 0.25, 0.75))
CustomFun(m, g=lambda t: t[..., 0] ** 2)
returning
qmcpy.util.exceptions_warnings.ParameterError: The range of the composed transform is not compatible with this true measure
-
One question I have is whether changing the clipping bound in
_clip_unit_interval()inqmcpy/true_measure/abstract_true_measure.pyfromnp.finfo(float).epstonp.finfo(float).tinywill be better. -
After all the bugs or issues are resolved, please consider updating the documentation in code and/or demo, for example, inconsistent labels 3A vs 5.1 in the demo. It will also be good to request re-review from @fjhickernell and @alegresor .
|
Thank you. I rechecked these cases more deeply. For # 1, I found that the discrepancy comes from an existing difference in how chained Before changing that behavior, could you confirm whether ordinary For # 2, I found the remaining unprotected quantile paths in I will also clean up the demo labels once the behavior is settled. |
Co-authored-by: fjhickernell <817530+fjhickernell@users.noreply.github.com>
6da68df to
345bb0d
Compare
|
Thank you for catching these. I corrected the constructor documentation in I also re-ran the following cases:
Items 1 and 3 Could you please confirm whether ordinary chained |
There was a problem hiding this comment.
Sorry for the delay in my response. I would think yes (chained TrueMeasures should consistently represent the full recursive transformation when used with an integrand) and have importance sampling as an explicit ImportanceSampling(target=, proposal=) class.
Could you weigh in before @Laasya-73 continues to morph this PR?
Summary
This PR generalizes compatibility checking for chained
TrueMeasuretransformations.Previously, consecutive transformations were considered compatible only when the range of the preceding transformation exactly matched the domain of the next:
This is more restrictive than necessary. The updated rule accepts the chain whenever the preceding range is contained within the next domain:
This allows valid chained transformations with strict domain inclusion while continuing to reject genuinely incompatible ranges.
Changes
_range_in_domain()toAbstractTrueMeasure.(1, 2)and(d, 2)domain/range representations.demos/true_measure_domain_inclusion.ipynbdemonstrating:TrueMeasurechains;Scope
This change is intentionally limited to domains/ranges represented as one-dimensional intervals or multidimensional axis-aligned boxes.
It does not introduce arbitrary measure-transport semantics, disconnected support representations, or changes to the existing importance-sampling implementation.
Notes
This PR replaces #602 after moving the feature branch into the
QMCSoftwarerepository.