Skip to content
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

Bindings for multi-index methods, factory methods and bindings for sigmoids, new 1d map #386

Merged
merged 27 commits into from
Feb 28, 2024

Conversation

dannys4
Copy link
Contributor

@dannys4 dannys4 commented Feb 7, 2024

Builds on #384 #385 #383 for ultimate user experience.

  • All new class✨:UnivariateExpansion is exactly what it promises. A quick and dirty 1d expansion which just trusts you to use a monotone 1d basis (see where concepts/constraints could come in yet?)
  • Fresh bindings✨: Bind DiagonalCoeffIndices for users to see which coefficients correspond to multivariate basis functions varying in $x_j$ for the $j$th input. Doesn't currently distinguish between inputs. This will be pushed off to a new PR due to changes in Added Diagonal Coefficient Functionality #383 .
  • New factory methods✨: CreateSigmoidComponent and CreateSigmoidTriangular mimic their non-sigmoid parts, except they take in (fixed) parameters for where the centers of the sigmoids should be. They also automatically choose the bandwidth of the sigmoids according to a heuristic (TODO: allow the user to choose this from the bindings). As far as I understand, where you place the centers generally makes a larger difference than how wide the sigmoids are, within reason.

I tested all the bindings locally in all the languages, and everything seemed to work. I'm hoping that this closes out the slew of PRs that leads to MParT 3.0, though some documentation may be in order.

@dannys4
Copy link
Contributor Author

dannys4 commented Feb 7, 2024

A very good criticism to consider that I just thought of is that there's no clear way here to get from Multi-index set to sigmoid component, which makes adaptive methods tricky. Due to the structure of the rectified MVE, you actually need two msets (satisfying some conditions), and so I'm not sure how accessible I should make this. Maybe I'll add a secondary backup method that takes two fixed msets?

@dannys4 dannys4 mentioned this pull request Feb 16, 2024
@dannys4 dannys4 mentioned this pull request Feb 23, 2024
@dannys4 dannys4 marked this pull request as ready for review February 27, 2024 12:36
Copy link
Contributor

@mparno mparno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work Danny! Another great addition. There is a matlab test file that looks like it might have snuck in, but everything else looks good to me.

bindings/matlab/tests/test-results-matlab.xml Outdated Show resolved Hide resolved
@dannys4 dannys4 requested a review from mparno February 27, 2024 21:24
@mparno mparno merged commit c6628cd into MeasureTransport:main Feb 28, 2024
5 checks passed
@dannys4 dannys4 deleted the newBindings branch April 2, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants