Implementing original qp PDF constructor #150

drewoldag · 2023-01-25T00:07:14Z

Modified the quantile distribution class to allow users to select from a variety of pdf reconstructors to best support their use case. The pdf construction algorithms include the original qp cdf spline derivative, a new average-of-splines approach, piecewise linear, and piecewise constant.

This branch also includes a new demo notebook with examples of how the quantile distribution class works, how to change pdf constructors, and includes a variety of example input distributions that are approximated by quantile distribution, and reconstructed with each of the different pdf construction algorithms.

…algorithms.

…LSSTDESC/qp into issue/8/original-qp-exploration

…algorithms.

…r switching and using constructors in quant_gen.

…is finished.

…as been started.

…ailed due to removing `quant_piecewise`

drewoldag · 2023-01-25T00:13:52Z

When I run unit tests locally I see the following tests failing:

FAILED tests/qp/test_auto.py::PDFTestCase::test_convert_quant_0 - ValueError: convert -1.91e+00 9.50e-01 {'atol': 0.39999999999999997}
FAILED tests/qp/test_auto.py::PDFTestCase::test_convert_quant_1 - ValueError: convert -1.91e+00 9.50e-01 {'atol': 0.39999999999999997}
FAILED tests/qp/test_auto.py::PDFTestCase::test_pdf_quant - ValueError: cdf -1.47e-02 8.88e-01 {'atol': 0.2}

…or so that unit tests continue to pass.

codecov · 2023-01-25T22:06:36Z

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (3c4f283) compared to base (0a5d77d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #150   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        31    +6     
  Lines         2047      2117   +70     
=========================================
+ Hits          2047      2117   +70

Flag	Coverage Δ
unittests	`100.00% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/qp/metrics/pit.py	`100.00% <100.00%> (ø)`
src/qp/quant_pdf.py	`100.00% <100.00%> (ø)`
src/qp/quantile_pdf_constructors/__init__.py	`100.00% <100.00%> (ø)`
...ntile_pdf_constructors/abstract_pdf_constructor.py	`100.00% <100.00%> (ø)`
...quantile_pdf_constructors/cdf_spline_derivative.py	`100.00% <100.00%> (ø)`
...p/quantile_pdf_constructors/dual_spline_average.py	`100.00% <100.00%> (ø)`
...qp/quantile_pdf_constructors/piecewise_constant.py	`100.00% <100.00%> (ø)`
...c/qp/quantile_pdf_constructors/piecewise_linear.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

drewoldag · 2023-01-25T22:37:59Z

src/qp/quant_pdf.py

-
-
-
-class quant_piecewise_gen(Pdf_rows_gen):


Completely removed this class because it is now superfluous. It was originally created so that an alternative pdf reconstruction algorithm could be implemented. Now that the pdf reconstruction algorithms are separated into their own classes and selectable at run time, we no longer need this class.

drewoldag · 2023-01-25T22:40:56Z

src/qp/quant_pdf.py

@@ -64,6 +72,13 @@ def pad_quantiles(quants, locs):

    return quants_out, locs_out

+DEFAULT_PDF_CONSTRUCTOR = 'piecewise_linear'


Selecting this PDF constructor as the default for now, only because there are unit tests with hard coded quant_gen.pdf() output values that will fail if we use a different constructor.

It would be preferable to use cdf_spline_derivative, however, I would like to defer that work to be done by (or in collaboration with) a researcher.

drewoldag · 2023-01-25T22:42:03Z

src/qp/quant_pdf.py

@@ -136,12 +144,63 @@ def locs(self):
        """Return the locations at which those quantiles are reached"""
        return self._locs

+    @property
+    def pdf_constructor_name(self):


This and the following three new methods are the machinery that allow the user to dynamically select pdf constructors at run time.

…onstruction algorithms.

…t around forever.

…tebook into the demo notebook in a "Reference" section at the end.

src/qp/quant_pdf.py

src/qp/quantile_pdf_constructors/abstract_pdf_constructor.py

delucchi-cmu · 2023-01-26T13:36:09Z

src/qp/quantile_pdf_constructors/cdf_spline_derivative.py

+        # ! create an issue (or fix) if the spline fit fails, can fall back to a simpler interpolator ???
+        self._interpolation_functions = [
+            InterpolatedUnivariateSpline(self._locations[i,:], self._quantiles, k=spline_order, ext=1).derivative()
+            for i in range(0,number_of_locations)


Does this line do anything?

Yeah, while it doesn't look like it, it does. Lines 51-54 represent a list comprehension spread out over several lines to try to make it more readable.

Usually list comprehension is a single line like:
output = [do_thing(i) for i in list_of_i_values]

But here, do_thing(i) is a long line, and I felt like it was more readable if it was split up.

src/qp/quantile_pdf_constructors/dual_spline_average.py

src/qp/quantile_pdf_constructors/piecewise_constant.py

tests/qp/quantile_pdf_constructors/test_cdf_spline_derivative.py

tests/qp/test_ensemble.py

…to dual_spline_average and added a test. Updated the demo notebook.

delucchi-cmu

Someone else should probably check the math, but the structure looks good!

sschmidt23

I think things look good, two minor points, though:
If you want to keep the temp_quantile_parameterization_under_the_curve_impl.ipynb notebook, maybe rename without the "temp" in the notebook name.

In demo.ipynb it looks like there's a problem with the pdf/cdf/ppf/etc... when the demo creates a single qp.quant_gen distribution with a single set of quants and locs rather than an ensemble. Calling .pdf .ppf .cdf throws a

TypeError: _parse_args() missing 1 required positional argument: 'row'

error from scipy, i.e. there is no row argument from a single distribution rather than an ensemble ( a qp.quant Ensemble with a single distribution works fine, though). We should fix this or update the demo notebook so that it executes.

… class.

sschmidt23

Looks good, thanks Drew!

drewoldag and others added 9 commits January 19, 2023 10:25

Initial WIP commit.

f2015fa

added a cell checking the CDF of reconstruction

8044bd8

WIP notebook showing spline and interpolated histogram interpolation …

498e9ac

…algorithms.

Merge branch 'issue/8/original-qp-exploration' of https://github.com/…

054264c

…LSSTDESC/qp into issue/8/original-qp-exploration

WIP notebook showing spline and interpolated histogram interpolation …

9f94b69

…algorithms.

WIP commit. Added skeletons for the pdf constructors and machinery fo…

9d8e128

…r switching and using constructors in quant_gen.

Added constructor exploration notebook. To be deleted after the work …

5b5b4fa

…is finished.

WIP commit. Constructors are mostly finished, and the demo notebook h…

8f68fe7

…as been started.

Added unit tests for the constructors. Updated some unit tests that f…

f27ea61

…ailed due to removing `quant_piecewise`

drewoldag self-assigned this Jan 25, 2023

drewoldag requested review from aimalz and OliviaLynn January 25, 2023 00:07

Fleshed out the example notebook, changed the default PDF reconstruct…

2c6ce47

…or so that unit tests continue to pass.

drewoldag marked this pull request as ready for review January 25, 2023 22:13

Added a couple more unit tests for full coverage.

c5b28e5

drewoldag commented Jan 25, 2023

View reviewed changes

drewoldag linked an issue Jan 25, 2023 that may be closed by this pull request

Piecewise non-constant quantile implementation #8

Closed

drewoldag added 3 commits January 25, 2023 14:56

Removed exploratory notebooks used to investigate the various PDF rec…

8cc8413

…onstruction algorithms.

Renamed notebook to temp_* to indicate that it is not meant to be kep…

417214a

…t around forever.

Copied the original qp implementation out of the "under_the_curve" no…

5dbf3fd

…tebook into the demo notebook in a "Reference" section at the end.

drewoldag mentioned this pull request Jan 26, 2023

Multimodal PDFs in the interpolated quantile parameterization #65

Closed

delucchi-cmu reviewed Jan 26, 2023

View reviewed changes

drewoldag added 2 commits January 26, 2023 11:24

Addressed PR comments.

b99e948

Updated quant_gen to persist pdf_constructor_name. Reverted a change …

20e66d7

…to dual_spline_average and added a test. Updated the demo notebook.

delucchi-cmu approved these changes Jan 27, 2023

View reviewed changes

sschmidt23 self-requested a review January 27, 2023 22:08

sschmidt23 requested changes Jan 27, 2023

View reviewed changes

Remove temporary notebook and lingering references to quant_piecewise…

3c4f283

… class.

sschmidt23 self-requested a review January 27, 2023 22:59

sschmidt23 approved these changes Jan 27, 2023

View reviewed changes

sschmidt23 merged commit be6954a into main Jan 27, 2023

sschmidt23 deleted the issue/8/original-qp-exploration branch January 27, 2023 23:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing original qp PDF constructor #150

Implementing original qp PDF constructor #150

drewoldag commented Jan 25, 2023

drewoldag commented Jan 25, 2023

codecov bot commented Jan 25, 2023 •

edited

Loading

drewoldag Jan 25, 2023

drewoldag Jan 25, 2023

drewoldag Jan 25, 2023

delucchi-cmu Jan 26, 2023

drewoldag Jan 26, 2023

delucchi-cmu left a comment

sschmidt23 left a comment

sschmidt23 left a comment

		@@ -64,6 +72,13 @@ def pad_quantiles(quants, locs):

		return quants_out, locs_out

		DEFAULT_PDF_CONSTRUCTOR = 'piecewise_linear'

Implementing original qp PDF constructor #150

Implementing original qp PDF constructor #150

Conversation

drewoldag commented Jan 25, 2023

drewoldag commented Jan 25, 2023

codecov bot commented Jan 25, 2023 • edited Loading

Codecov Report

drewoldag Jan 25, 2023

Choose a reason for hiding this comment

drewoldag Jan 25, 2023

Choose a reason for hiding this comment

drewoldag Jan 25, 2023

Choose a reason for hiding this comment

delucchi-cmu Jan 26, 2023

Choose a reason for hiding this comment

drewoldag Jan 26, 2023

Choose a reason for hiding this comment

delucchi-cmu left a comment

Choose a reason for hiding this comment

sschmidt23 left a comment

Choose a reason for hiding this comment

sschmidt23 left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 25, 2023 •

edited

Loading