Change _topods_entities

[jdegenstein]

Preliminary testing show the new version has slightly better performance. According to several sources the returned order when using TopTools_IndexedMapOfShape and TopExp.MapShapes_s is perfectly repeatable -- unlike the existing version. I believe the one test that was failing is a result of the topology being explored in a different order, which according to what I have read, can affect the forward/reverse properties of TopoDS shapes.

I still would like a MacOS arm64 user to test this and confirm performance is not significantly worse (the macos github runners have extremely variable performance). Opening as a draft PR for now as a result of this uncertainty.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.57%. Comparing base (1b5688a) to head (55ad15a).
Report is 9 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #873      +/-   ##
==========================================
+ Coverage   96.38%   96.57%   +0.19%     
==========================================
  Files          31       31              
  Lines        9120     9120              
==========================================
+ Hits         8790     8808      +18     
+ Misses        330      312      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jdegenstein]

Received from benchmarks for macos arm64 and this is probably slightly faster than the older algorithm -- but more importantly it is definitely not significantly slower on all platforms This is ready for review.

[gumyr]

TopExp_Explorer is directly used in quite a few modules, should they all be changed over to this method?

During the refactor I followed the number of function calls in edges - here is what I found:
1- Mixin1D.edges(self) calls Shape.get_shape_list(self, "Edge")
2- Shape.get_shape_list(self, "Edge") calls shape.entities(entity_type)]
3- shape.entities(entity_type)] calls _topods_entities(self.wrapped, topo_type)
4- _topods_entities(self.wrapped, topo_type) calls TopExp_Explorer
5- TopExp_Explorer causes a dict lookup from str to a TopAbs ENUM (e.g. ta.TopAbs_VERTEX) which could have been given to at the start.

Maybe we should look at optimizing selector methods given how frequently they are used?

[jdegenstein]

Yes, there is a lot of room to reduce the layers of function calls here and optimize the various type conversions that occur. Should I merge this and we can proceed with that separately?

[gumyr]

What about the other uses of TopExp_Explorer - should they be eliminated as well?

[jdegenstein]

FYI I have been working on this, refactored most of the places directly using TopExp_Explorer to instead use _topods_entities. I am still looking at optimizing the overall flow of function calls as you described.