Geometry_Engine: Optimisation and tweaks

[pawelbaran]

Issues addressed by this PR

Closes #3259
Closes #3261

Test files

On SharePoint.

Changelog

Additional comments

Apologies for the massive brain dump and a few different topics covered, but this is all stuff I kept on tweaking while working on various things - happy to split it up into smaller PRs if requested by @peterjamesnugent (volunteered to review) or anyone else eager to have a look.

[pawelbaran]

@BHoMBot check compliance

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check branch-compliance
• check dataset-compliance
• check copyright-compliance

[pawelbaran]

@BHoMBot check unit-tests

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check unit-tests

[pawelbaran]

@BHoMBot check null-handling
@BHoMBot check core
@BHoMBot check serialisation

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check null-handling
• check core
• check serialisation

[pawelbaran]

@BHoMBot check versioning
@BHoMBot check installer

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check versioning
• check installer

[pawelbaran]

@BHoMBot check versioning

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check versioning

There are 1 requests in the queue ahead of you.

[pawelbaran]

@BHoMBot check required

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

There are 1 requests in the queue ahead of you.

[bhombot-ci]

The check versioning has already been run previously and recorded as a successful check. This check has not been run again at this time.

[peterjamesnugent]

Happy with changes to FitLine, test script works great and tested with the script I added to the issue too.

Happy with Colinear and Coplanar, tested using the script.

Happy with RoundCoordinates, BooleanDifference, BooleanIntersection and BooleanUnion working as intended, tested using the script.

The unit tests will also demonstrate that the methods have not changed (just extended functionality and dealing with edge cases).

One question I had was on tolerance.

• For IsColplanar the method is true when the difference is 6e-6, but tolerance input is only 1e-6;
• For IsColinear the method is true when the different is 3e-6 but tolerance input is only 1e-6;

Are we stacking the tolerances as we feed them through methods and should we be dividing them?

I think we should be consistent with how we refer to List<T>, in some places you refer to it in the description as a set and in other places its a collection. I think I prefer set or list - but happy for others to chime in.

[pawelbaran]

Thanks @peterjamesnugent for the review. I made the tweaks recommended in the code comments.

Regarding the tolerance issues - it is a much bigger problem that boils down to the fact that line fitting, plane fitting, collinearity and coplanarity checks are numerical algorithms rather than geometrical, i.e. the geometrical tolerance cannot be applied 1:1. Long time ago I have spent some time trying to crack the problem, outcome being Compute.REFTolerance, but it is still pretty far from perfect.

Happy to bring back the discussion if you think the current precision is not good enough. But it is definitely beyond the scope of this PR 😉

[pawelbaran]

@BHoMBot check required

[bhombot-ci]

@pawelbaran to confirm, the following actions are now queued:

• check code-compliance
• check documentation-compliance
• check project-compliance
• check core
• check null-handling
• check serialisation
• check versioning
• check installer

There are 2 requests in the queue ahead of you.

[FraserGreenroyd]

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance

[bhombot-ci]

@FraserGreenroyd to confirm, the following actions are now queued:

• check copyright-compliance
• check dataset-compliance

There are 4 requests in the queue ahead of you.

[peterjamesnugent]

Happy that the last commit was all descriptions and making use of the IsNull, so my previous review of functionality is still valid.

I assume there will be a seperate PR to update unit tests to include (at least some) of the cases in the test scripts.

[FraserGreenroyd]

@BHoMBot this is a DevOps instruction. I am requesting neutral checks on: unit-tests

[bhombot-ci]

@FraserGreenroyd I have provided neutral checks to the checks requested. These checks will need to be run properly to obtain full results.

[peterjamesnugent]

@BHoMBot check ready-to-merge

[bhombot-ci]

@peterjamesnugent to confirm, the following actions are now queued:

• check ready-to-merge