Skip to content

Add all the remaining #define macros from predicates.c #7

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

Merged
merged 6 commits into from
Jul 10, 2024
Merged

Add all the remaining #define macros from predicates.c #7

merged 6 commits into from
Jul 10, 2024

Conversation

DanielVandH
Copy link
Member

@DanielVandH DanielVandH commented Jul 9, 2024

The remaining arithmetic functions still need to be added but here I've added all the remaining #define macros.

  • To test them all properly, I rewrote the #define macros as functions in C so that we can get them through @ccall. I tested my conversions in C in macro_defs.c. I've pretty much never used C (only one other time for ~20 minutes before looking into this PR) so my C code is probably very rough.
  • I then test the Julia versions by comparing to the @ccall. For some reason the only one not exactly correct all the time is Two_Square. I don't know why exactly...

I restructured the code a bit to help organise it better when I later add the remaining functions.

(The point here being of course that I'm trying to get all the macros/arithmetic functions in and tested so I can port over the remaining predicates)

@DanielVandH
Copy link
Member Author

For some reason the only one not exactly correct all the time is Two_Square. I don't know why exactly...

Could be related to this comment

/* An expansion of length two can be squared more quickly than finding the /
/
product of two different expansions of length two, and the result is /
/
guaranteed to have no more than six (rather than eight) components. */

in the code. Maybe my tests are breaking the expansion of length two assumption.

@DanielVandH
Copy link
Member Author

Actually, Two_Square isn't even used anywhere in predicates.c.. I'll just remove it. This is good to go.

@DanielVandH DanielVandH merged commit ae0d28b into JuliaGeometry:main Jul 10, 2024
3 of 9 checks passed
@DanielVandH DanielVandH deleted the allmacros branch July 10, 2024 08:24
@DanielVandH
Copy link
Member Author

Actually, Two_Square isn't even used anywhere in predicates.c.. I'll just remove it. This is good to go.

Fixed! #12 - it was a bug in the original code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant