-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
CI failure on Windows #2376
Comments
The Windows part of the CI build has been disabled in #2377. |
I have been investigating thru the code, and I think that there is actually 2 points in almost equal coords, and for some reason the Windows version has more precision than Linux?, for that reason panics an the others don't?. On Windows the "holes" model also panics, and trhought testing is caused by both "add hole" functions (trhu and blind). Maybe it will be interesting to implement a way to view all points and curves in a model for debugging purposes, handling panics. |
Thank you for looking into this, @Sergi-Ferrez!
That's what I thought too, initially, but there's a problem with that explanation. Here's the code that does the comparison and then panics: fornjot/crates/fj-core/src/algorithms/approx/face.rs Lines 47 to 55 in e67dc74
As you can see, it compares against
And the value itself is defined here:
So the minimum distance value below which two points are considered the same (leading to the panic) is But compare that to the actual distance reported in the error message, If the actual distance was close to the minimum distance, I'd believe it's just a difference in precision between platforms, so that one platform ends up above the threshold, the other below it. But the difference is so large, that I find this hard to believe. Which is why I concluded that it must be a logic error. But all that said, as I said in the issue description, I have not idea how such a logic error could occur just on Windows, so maybe I'm totally wrong in my analysis.
Yeah, that would be awesome. Overall, the tooling we have for debugging issues like that isn't very good, unfortunately. |
As of #2375, the CI build is broken on Windows: https://github.com/hannobraun/fornjot/actions/runs/9405591496/job/25907555438?pr=2375
It fails in the testing job, with a panic from the approximation code:
The code in question compares a distance against a tolerance value, so my first assumption was, that this is a case of the different platforms having different floating point precision. However, the distance and the approximation value are nowhere close, so I don't think it can be that.
And that's what confounds me about this failure. It looks like a logic error, but I have no idea how such a logic error could occur just on Windows. This doesn't reproduce on my machine (and I don't have a Windows to test), and I don't think trying to solve this using the CI build would be a productive use of my time right now (the slow turnaround times would make it extremely tedious and time-consuming). Therefore, I will disable the Windows CI build for the time being.
I'm tagging this with help wantedExtra attention is needed
, as I hope that someone will be able to reproduce this locally sooner or later, and will then have a much easier time tracking this down than I would have.
The text was updated successfully, but these errors were encountered: