[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/681 Can you share some use cases where you find `NAN` useful to have as a keyword? What itch is this scratching? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/681 @cestella and I were talking about null variables and how NaN was being used, and that we probably needed NaN as a keyword. So I took a stab at it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user cestella commented on the issue: https://github.com/apache/metron/pull/681 Some of our math functions return `NaN` (e.g. `SQRT(-1)`) and being able to do `!= NaN` is a useful thing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user cestella commented on the issue: https://github.com/apache/metron/pull/681 Just thinking, we might want +Infinity and -Infinity too as they're part of the floating point standard. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/681 Ok, that makes sense to me. `FUNCTION(x) != NaN` The code changes look good to me @ottobackwards. I am going to pull it down and play with NaN in the REPL just as a sanity check. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/681 I see in the unit tests that @ottobackwards intended that any equality with `NaN` to be false. But then how is that useful for the basic use case of `FUNCTION(x) != NaN`? ``` [Stellar]>>> SQRT(-1) NaN ``` I would expect the following to return `true`. ``` [Stellar]>>> SQRT(-1) == NaN false ``` I would expect the following to return `false`. ``` [Stellar]>>> SQRT(-1) != NaN true ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/681 Based on the link that you embedded in the tests, your implementation seems correct. I assume that these grammar rules are well reasoned and necessary, but it surely seems counter-intuitive to me for this use case. > The equality operator == returns false if either operand is NaN. > The inequality operator != returns true if either operand is NaN (§15.21.1). > In particular, x!=x is true if and only if x is NaN, and (x=y) will be false if x or y is NaN. Am I thinking about this all wrong? Maybe I just need some lunch to get my blood sugar up. :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/681 Technically this is correct, as for example SQRT(-1) and SQRT(-2) clearly should not show as equal, but by using NaN we (deliberately) lose the info that would allow us to recognize that SQRT(-1)==SQRT(-1). To detect/validate NaN, we would need an operator IS_NAN(x). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/681 We are not 'doing' the math here. What we are doing is introducing the ability to use NaN in an expression correctly. Java is doing the math. The tests are to verify that through stellar, the specified behavior still is adhered to. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/681 BTW, +/- Infinity **are** considered "real" values (essentially as tho via rounding) and can be compared with equality and inequality. NaN is considered a nonsensical or impossible-to-calculate value, not a number at all. The fact that 1.0/0.0 returns positive Infinity rather than NaN is a mathematical choice, presumably based on limit logic and a preference for continuity. (I didn't write the standard! :-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/681 @ottobackwards , the validator for IS_NAN(x) in Java would be Double.isNaN(double x). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/681 Thanks for clarifying @mattf-horton . That makes sense now that I've had lunch. Based on the original use case of detecting a function returning NaN, it seems what we really need is something like `IS_NAN(x)` rather than a keyword for `NaN`. Implementing `IS_NAN(x)` doesn't technically require a keyword for `NaN`. But I can see a potential use for the `NaN` keyword when creating a lambda expression. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user cestella commented on the issue: https://github.com/apache/metron/pull/681 Yeah, it's true, I suppose having `==` double as a `NaN` check is probably not the right thing due to the transitivity of equality (I'd like to `SQRT(-1) == NaN` to be `true` but `SQRT(-1) == SQRT(-2)` to not be true). Probably it's better to have an `IS_NAN(x)` like other languages. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/681 @nickwallen is right, since they can't be usefully compared, the only use for a literal NaN is if you want to inject that value into a calculation, which as far as I can see is only useful for unit testing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/681 I have added IS_NAN() --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/681 @ottobackwards Could you deconflict this? It seems like semantic conversation died down and hopefully we're at least mostly set since `IS_NAN()` got added. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/681 done ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/681 @cestella and @mattf-horton Do we care about the Infinity cases and presumably an `IS_INFINITE()` in this PR, or are we okay with holding off until a follow on is made? Other than that question, and one test case I'd like added, I'm happy with this. Thanks for the contribution! ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/681 I'm on the side of holding off for an eventual follow-on regarding that question, for the record. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user cestella commented on the issue: https://github.com/apache/metron/pull/681 Yeah, I'm ok with that. ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/681 I'm +1, thanks again for the contribution! ---
[GitHub] metron issue #681: METRON-1079 Add NaN as a keyword in STELLAR language
Github user cestella commented on the issue: https://github.com/apache/metron/pull/681 me too, +1 ---
