Always check in the code for explicit parallel/serial choice #980
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #980 +/- ##
==========================================
- Coverage 91.84% 88.21% -3.63%
==========================================
Files 37 62 +25
Lines 4976 8670 +3694
Branches 0 1044 +1044
==========================================
+ Hits 4570 7648 +3078
- Misses 406 1022 +616 ☔ View full report in Codecov by Sentry. |
|
Nice. Maybe we want to do this to other flags as well? e.g. MANIFOLD_DEBUG. @elalish what do you think? |
Yeah, actually it's probably a good idea to be uniform about this. This is basically to catch our own CMake errors, right? And those could happen with any flag, so let's make this part of our coding style. |
|
The thing about doing it for the debugging flag... I would expect, by default, user codes wouldn't turn debugging on - so it may feel a little weird to them to have to explicitly say it is off. Parallel I think makes sense because of the importance - if you try to turn parallel on or off and get it wrong the consequences can be severe - but we might not want to force client codes to set a large number of defines to build. |
|
The other alternative would be to put the compile failing checks for the define in the internal C++ files - right now I've got it in the public header. If we want this to be a for-manifold-compile-only thing, I should push it down to the internal files. |
We want this check for Manifold compilation as a cross check on our CMake logic, but this way client codes will work by default without being forced to set it explicitly.
|
Yeah, putting it in our internal files will be good enough. |
|
Probably should do the debug flag in a separate PR? |
…#980) * Check at compile time that MANIFOLD_PAR is set to either P (parallel) or S (series) mode. * Switch from 'P' and 'S' to 1 and -1 * fix formatting * Put the MANIFOLD_PAR poison pill in an internal header. We want this check for Manifold compilation as a cross check on our CMake logic, but this way client codes will work by default without being forced to set it explicitly.
In order to avoid accident serial (or parallel, for that matter) compilation settings, always check in the code for an explicit and valid MANIFOLD_PAR value.