BHoM / BHoM_Engine

Internal manipulation of the BHoM
GNU Lesser General Public License v3.0
26 stars 13 forks source link

Geometry_Engine: Optimisation and tweaks #3275

Closed pawelbaran closed 7 months ago

pawelbaran commented 7 months ago

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 commented 7 months ago

@BHoMBot check compliance

bhombot-ci[bot] commented 7 months ago
@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 commented 7 months ago

@BHoMBot check unit-tests

bhombot-ci[bot] commented 7 months ago
@pawelbaran to confirm, the following actions are now queued: - check `unit-tests`
pawelbaran commented 7 months ago

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

bhombot-ci[bot] commented 7 months ago
@pawelbaran to confirm, the following actions are now queued: - check `null-handling` - check `core` - check `serialisation`
pawelbaran commented 7 months ago

@BHoMBot check versioning @BHoMBot check installer

bhombot-ci[bot] commented 7 months ago
@pawelbaran to confirm, the following actions are now queued: - check `versioning` - check `installer`
pawelbaran commented 7 months ago

@BHoMBot check versioning

bhombot-ci[bot] commented 7 months ago
@pawelbaran to confirm, the following actions are now queued: - check `versioning` There are 1 requests in the queue ahead of you.
pawelbaran commented 7 months ago

@BHoMBot check required

bhombot-ci[bot] commented 7 months ago
@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[bot] commented 7 months ago
The check `versioning` has already been run previously and recorded as a successful check. This check has not been run again at this time.
pawelbaran commented 7 months ago

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 commented 7 months ago

@BHoMBot check required

bhombot-ci[bot] commented 7 months ago
@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 commented 7 months ago

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

bhombot-ci[bot] commented 7 months ago
@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.
FraserGreenroyd commented 7 months ago

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

bhombot-ci[bot] commented 7 months ago
@FraserGreenroyd I have provided neutral checks to the checks requested. These checks will need to be run properly to obtain full results.
peterjamesnugent commented 7 months ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 7 months ago
@peterjamesnugent to confirm, the following actions are now queued: - check `ready-to-merge`