CG-SHOP / pyutils25

Utilities for verifying solutions of the CG:SHOP 2025 Competition.
MIT License
0 stars 5 forks source link

Negative numbers are not correctly validated #6

Closed JacobusTheSecond closed 1 month ago

JacobusTheSecond commented 1 month ago

Some instances (most orthogonal instances) require negative coordinates for steiner-points.

When validating the input to the Cgshop2025Solution Model (in src/cgshop2025_pyutils/data_schemas/solution.py) it is checked if all coordinates are rational

        def is_rational(value):
            if isinstance(value, int):
                return True
            if isinstance(value, str):
                if "/" in value:
                    components = value.split("/")
                    if len(components) != 2:
                        return False
                    a, b = components
                    return a.isdigit() and b.isdigit()
                return value.isdigit()
            return False

However, negative integers (or fractions) are incorrectly marked as not rational, as isdigit() only checks if every character is a digit:

> "-10".isdigit()
False
d-krupke commented 1 month ago

Thanks for letting us know. The generators were actually supposed to only generate positive coordinates, but it seems like there was miscommunication in our group. I extended the scheme and it should now work. Please let me know if there are still any issues :)

JacobusTheSecond commented 1 month ago

Heyhey, thanks for fixing it this quickly!

There seems to be a follow-up problem in the FieldNumber binding (also with negative numbers), namely if I generate a negative FieldNumber of length exactly 15 (or more precisely 14*i +1). This stems from the file src/cgshop2025_pyutils/geometry/_bindings.cpp, as negative numbers of specific lengths get split, it may happen, that one split (small_part) contains all digits, and the other split (large_part) contains only the string "-", which is not a valid number.

Kernel::FT str_to_exact(std::string number) {
  number.erase(0, number.find_first_not_of('0'));
  if (number.empty())
    number += '0';
  if (std::count(number.begin(), number.end(), '/') == 1) { // rational numbers
    auto point_pos = number.find('/');
    auto numerator = str_to_exact(number.substr(0, point_pos));
    auto denominator = str_to_exact(number.substr(point_pos + 1));
    return numerator / denominator;
  }
  constexpr size_t max_len = 14;
  if (number.length() <= max_len) {
    return to_exact(std::int64_t(std::stoll(number)));
  }
  auto small_part = str_to_exact(number.substr(number.length() - max_len));
  auto large_part = std::pow(10, max_len) *
                    str_to_exact(number.substr(0, number.length() - max_len));         //here
  return large_part + small_part;
}

std::string point_to_string(const Point &p) {
  return fmt::format("({}, {})", CGAL::to_double(p.x()),
                     CGAL::to_double(p.y()));
}

Minimal breaking example:

from cgshop2025_pyutils.geometry import FieldNumber
FieldNumber("-10000000000000")

throws

ValueError: stoll: no conversion
d-krupke commented 1 month ago

Good catch! Will take care of it!

d-krupke commented 1 month ago

Fix is up, the CI will take a few minutes to publish it.