DMN 1.4 - 1141-feel-round-up-function

[opatrascoiu]

@StrayAlien Thank you for the tests.

Before we start reviewing the rounding tests here is the background of the proposal:

• the goal of the new round functions I submitted to RTF was to have support for all the available rounding modes. Prior to the proposal DMN already had support for certain rounding modes, see decimal(), floor(), and ceiling() functions.
• the semantics is identical to https://docs.oracle.com/javase/8/docs/api/java/math/RoundingMode.html. If the spec does not cover all cases (e.g. negative scales) we should follow the one above and add examples / working in spec.

AFAIK there are already test cases for decimal() that cover some edge cases (e.g. decimal(1/3, 2.5) where the expected behavior is to use only the int value of the scale). We need to align the new tests with the existing ones to treat the edge cases in the same way (e.g. negative scales).

Most of the test cases look good. So far, I spotted a few tests that need to be discussed: 0015, 0016_a, 0016_b, 0017_b. Please review them based on the above.

Thank you again.

[StrayAlien]

Hi @opatrascoiu - thanks for that link and the comments. Yes, let's discuss those tests for sure.

As there are quite a number of DMN 1.4 suites that dealing with scales, I'll open up a discussion so we have it all in a single place.

here: https://github.com/dmn-tck/tck/discussions/607

[baldimir]

The part where the scale of -6111 is mentioned in the spec is "Table 76: Semantics of numeric functions", after the table. There is written "Scale is in the range [−6111 ..6176]". We need to clarify with RTF, why these numbers.

[opatrascoiu]

@baldimir I raised two issues with RTF to discuss. See https://github.com/dmn-tck/tck/discussions/607

[baldimir]

Thank you @opatrascoiu.

[opatrascoiu]

The issue with the "Scale is in the range [−6111 ..6176]" has been clarified in RTF. The tests look good to me.

[SimonRinguette]

I would love to remove test case 006 and have the RTF add a version of the function without scale to align with ceiling and floor functions!

[baldimir]

After discussion, there was not an agreement if we want the test 006 in or not, so because of this, we should comment it out with a description why we did so.

[SimonRinguette]

RTF issue: https://issues.omg.org/issues/INBOX-1769

[StrayAlien]

test 6 commented out as per request

[SimonRinguette]

The 11b test that call using a named parameter (only one) should have also been removed with test 6 because it is the same test but with named parameter

[StrayAlien]

11b commented out