search

BHoM / BHoM_Engine

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

Base_Engine: Hash to leverage the new aggregating GeometryHash method for geometry types #3255

Closed alelom closed 7 months ago

alelom commented 7 months ago

Description:

This depends on #3251 .

Currently, the Hash method traverses all properties of geometric types to create their signature. This can be extremely slow, and can instead leverage the GeometryHash method(s). In particular, when we will have implemented the aggregating GeometryHash method that returns a single number (#3251), we can use it instead of this property traversal for geometric types in the main Hash method. We can expose this as the default behaviour, but also have an option in the ComparisonConfig to toggle the old property traversal behaviour for Geometric types, if required for some reason. Further, we can expose the GeometryHash optional arguments such as the translation tolerance as properties of the ComparisonConfig class.

This will greatly increase the speed (and accuracy) of the Hash method for geometric types. Further, we will be able to always use the main Hash method for all use cases – including geometric types. This would make things more consistent and simpler to use.

Thoughts? @pawelbaran @al-fisher

pawelbaran commented 7 months ago

Looks like the great minds think alike! 😃 I had the same thought when I first learnt about GeometryHash - why aren't we calling it from the Hash method?! So yes, 100% support, especially that I do not think there is any precedent where we would persist the hashes at the moment - most users compare on the fly, i.e. if what they think is the same still comes back as the same, it is not a breaking change 👍

alelom commented 7 months ago

I had the same thought when I first learnt about GeometryHash - why aren't we calling it from the Hash method

Mainly just because we've written GeometryHash after Hash to solve a specific problem, but it was always in consideration (at least for me) to have it called from there.

if what they think is the same still comes back as the same, it is not a breaking change

I agree we shouldn't consider it a breaking change. However, it could be considered breaking of workflows were the hash has been stored as text somewhere as a "reference signature" of a set of objects. I don't think the usage is yet wide enough to warrant a worry about such cases.

al-fisher commented 7 months ago

Yes - 100% 😸 Love this