-
Notifications
You must be signed in to change notification settings - Fork 1
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
Support for monomial computation (local evaluation) #72
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…otation in eval_poly (as it was changed sinc the original creation of eval_local)
…f rowProds to multipli the values of the variables selected by label. Also moved that part to an aux function.
… n_sample value to be provided in order to have a more general code that can receive both vectors and matrices.
…ent with each other
…f monomials==TRUE and otherwise sum them to obtain the final poly prediction.
…d references to monomials=TRUE output being reduce to a matrix if n_poly = 1, as we will always keep the 3D array output when we consider monomials.
…als are being used.
Enchufa2
approved these changes
Nov 21, 2024
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.
LGTM!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR closes #71, providing an option in
eval_poly()
andnn2poly.predict()
to compute the local evaluation of each monomial instead of the final polynomial, using an optional boolean parametermonomials
. Defaults toFALSE
.The new functionality has been finally included by changing the inner behavior of
eval_poly()
to store all the monomials in a 3D array as suggested in #71, and then at the end the sum of all monomial values is performed ifmonomials=FALSE
m, thus obtaining the final polynomial prediction. This may require more memory usage than before if there are too many polynomials or too many terms in them, but alternative implementations that were tested would require duplicating code or including too many if statements. This shouldn't be a problem in the usual applications, but if scaling becomes a problem we may need to revisit this implementation.Corresponding tests have been added and the documentation has been updated. Now the
nn2poly.predict()
output has several conditions that change its format and therefore a "long" explanation appears in the docs, but it seems to be as compact as possible. Vignette 01 has been also updated to include an extra paragraph and code block showing how to obtain the monomials with a given polynomial.Finally, a small discussion about
nn2poly.output()
might be needed: ignoring the case with several internal layers, the output is generally a matrix whenmonomials=FALSE
and a 3D array whenmonomials=TRUE
. However, when we have a single polynomial being evaluated, the output whenmonomials=FALSE
is reduced to a vector, as the matrix would have a single column. In the case ofmonomials=TRUE
and a single polynomial, I initially thought of reducing the dimension from a 3D array to a matrix because the last dimension would be 1 too. However, this lead to some problems as setting the output to beoutput[ , , 1]
creates a matrix if both remaining dimensions are greater than 1, or a vector if one of them is 1, which creates problems when trying to interact with the output in a general manner. Therefore, in the monomials case I decided to leave it as a 3D array always. This leaves two options:monomials=FALSE
be always a matrix and not make it a vector when there is only 1 polynomial. This was done like this to match other predict methods that provide a vector as output.