Skip to content
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

Geometry_Engine: GeometryHash() to support the options from BaseComparisonConfig #3269

Closed
alelom opened this issue Jan 25, 2024 · 0 comments · Fixed by #3268
Closed

Geometry_Engine: GeometryHash() to support the options from BaseComparisonConfig #3269

alelom opened this issue Jan 25, 2024 · 0 comments · Fixed by #3268
Assignees
Labels
type:feature New capability or enhancement

Comments

@alelom
Copy link
Member

alelom commented Jan 25, 2024

Description:

Because BaseComparisonConfig has the role of determining the configuration used when identifying an object (for comparisons), it should be provided as an input to all comparison and hashing methods. We need to introduce it in the GeometryHash() method and, consequently, in the HashArray() (formerly called "ToDoubleArray()") method.

This is particularly needed when we implement the support for using the GeometryHash method from the main Hash and Diffing methods. In fact, in order to add this support, we need to provide an new "UseGeometryHash" option to the BaseComparisonConfig (see BHoM/BHoM#1587). This last option needs to be orthogonal to the rest of the options in the BaseComparisonConfig, meaning, we need to make the rest of the options relevant even when setting "UseGeometryHash" to true.

An alternative would be to not implement the BaseComparisonConfig options in the GeometryHash method, but this means that, if we want to implement BHoM/BHoM#1587, the UseGeometryHash option would not be orthogonal to the rest of the options, requiring to make the BaseComparisonConfig IImmutable and introduce checks whenever it is used, in order to ensure that the combination of its property is consistent. For example, we'd not be able to allow a workflow where TypeExceptions includes a Geometric Type and UseGeometryHash is set to true.
This alternative however adds a lot of burden for existing downstream implementations, and risks regarding usage of Hash and Diffing, because ensuring consistency is not easy. After careful evaluation, I determined that it is more convenient to implement support for all of the BaseComparisonConfig options in the GeometryHash rather than going for this alternative. In fact, it is possible to add this support without impacting performance (only a minor impact is expected when the Config includes geometrical settings, e.g. if TypeExceptions include a Geometric type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant