-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added cubature recombination example. #798
Conversation
Adds a test to demonstrate the end-to-end usage of `TreeRecombination` for constructing non-product cubature formulae from product cubature formulae. A corresponding integration test has also been added for this new example. refs: #778
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few documentation comments - additionally the doc builds fail - this might be resolved simply by merging in main to this branch
@tc85324 Related to end-to-end usage of recombination, I'd be interested in your thoughts on how to sensibly include Recombination into the benchmarking currently being done on PR #802 . In that PR, @qh681248 is exploring how the various coreset methods compare in terms of run-time and error (MMD, KSD, ...) on a suite of candidate test problems. Since recombination isn't an |
Completes docstrings that were missed in a previous commit, simplifies some logic to aid in code interpretability, and corrects some typehints. Refs: #798
Performance reviewCommit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tc85324 Related to end-to-end usage of recombination, I'd be interested in your thoughts on how to sensibly include Recombination into the benchmarking currently being done on PR #802 . In that PR, @qh681248 is exploring how the various coreset methods compare in terms of run-time and error (MMD, KSD, ...) on a suite of candidate test problems. Since recombination isn't an
ExplicitSizeSolver
direct comparisons to coresets from other methods of a given size may not be in some sense fair. We could exclude it from benchmarking because of this - but if you have any ideas on sensible ways to include it we'd be open to suggestions.
Thank you for the review! Regarding benchmarking, I would say that the example in this pull is probably the canonical use case for recombination and it would be useful to have runtime performance figures for varying numbers of test-functions (dimension and degree), and in the case of TreeRecombination
, tree_reduction_factor
. While this won't be comparible to the ExplicitSizeSolvers
it is useful from an implementaiton perspective as we can check/see if real world performance scales at the same/simlar rates to those claimed in the literature.
Closes #778
PR Type
Description
Adds a test to demonstrate the end-to-end usage of
TreeRecombination
for constructing non-product cubature formulae from product cubature
formulae. A corresponding integration test has also been added for this
new example.
How Has This Been Tested?
The new tests pass locally and contain theoretically grounded assertions.
Does this PR introduce a breaking No
Checklist before requesting a review