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

SOLR-13391: Add variance and standard deviation stream evaluators #643

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@NazerkeBS
Copy link
Contributor

commented Apr 11, 2019

@joel-bernstein, please have a look to the code.

@risdenk

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Out of curiosity, can you wrap the new var evaluator with the sqrt evaluator to get stddev? Not sure we can use them together to make the new stddev evaluator.

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Out of curiosity, can you wrap the new var evaluator with the sqrt evaluator to get stddev? Not sure we can use them together to make the new stddev evaluator.

The Java implementations don't have a great way of doing that. You could do that easily with the expression language though. The java implementations assume that they are being passed information through the expression parser.

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

The implementations looks great. There are a couple places to add tests.
The MathExpressionTest.java has all the math expression tests. You can use testDotProduct as an example.

And TestLang.java just needs the new functions to be added to the allFunctions array.

To run the tests:

ant test -Dtestcase=MathExpressionTest
ant test -Dtestcase=MathExpressionTest -Dtests.method=testDotProduct

Thanks for the contribution!

@NazerkeBS

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I added a separate test for the variance and the standard deviation. The test passes.

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Here is how it looks from the Zeppelin-Solr interpreter
Screen Shot 2019-04-12 at 9 11 48 AM

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

This is pretty much ready to go. Thanks again for the contribution.

I should be able to get this in for the next release. It will be listed in the CHANGES.txt as:

SOLR-13391: Add variance and standard deviation stream evaluators (Nazerke Seidan, Joel Bernstein)

Let me know if you'd like your name listed differently.

@NazerkeBS

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I think, that is fine. In the future I would like to contribute more for streaming expressions as I am working on this topic.

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

I think, that is fine. In the future I would like to contribute more for streaming expressions as I am working on this topic.

Ok, ping me if you have a PR to look at. Also let me know if you want suggestions for things to work on.

@NazerkeBS

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I would be so grateful if you could give me a few suggestions to work on.

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Ok, this is a great ticket to work on:
https://issues.apache.org/jira/browse/SOLR-13047
I'll update it with some thoughts on how to get started.

@janhoy

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

@joel-bernstein you could close this PR, not?

@joel-bernstein

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2019

This can be closed, but I don't have permission to close it apparently. I also currently am not setup to merge PR's.

@ctargett

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

@joel-bernstein Have you linked your GH and ASF accounts? It's a simple procedure of adding your GH ID to your ASF profile: https://reference.apache.org/committer/github. Once its sync'd you should be able to fully edit/merge PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.