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

Added switch to replace simple pyramids and snouts with boxes and cyl… #66

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

minorai
Copy link
Collaborator

@minorai minorai commented Feb 16, 2022

…inders

This is Part 1, I am still working on tests for part 2 to cover all test cases and transforms.

@minorai minorai requested review from Strepto and espenrl February 16, 2022 17:38
@minorai
Copy link
Collaborator Author

minorai commented Feb 16, 2022

This should not have any effect on reveal pipeline, since we already do this in CadRevealComposer

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

Added missing box detection for facet groups. Tested on few thousand objects. Automated tests need to be written. Class needs some clean-up.

Suggestion for shared algebra functions? Maybe it is time to move them in RvmSharp out of CadRevealComposer

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

hda_optimized
HDA with optimization flag

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

hda_rotatedbox_from_facetgroups
FacetGroup to rotated box verification (both facet groups and boxes are displayed, only facet groups are selected)

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

hda_nonrotatedbox_from_facetgroup
FacetGroup to Box (no rotation) verification. Easy case

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

HDA:
Geometry primitives: 310306.
Original box count: 52316.
Original pyramid count: 30666
Original facet group count: 75079.
Pyramid To Box count: 27221 ( +52% boxes, 9% of all objects, 89% of all pyramids)
Snout To Cylinder count: 1049 (+1% cylinders)
Facet Group to Box count: 8986 (+17% boxes, 2% of all objects, 12% of all facet groups)

@Strepto
Copy link
Collaborator

Strepto commented Feb 17, 2022

Investigating the failing builds on WS1611 its related to the uploader having issues. I'll revert the Uploading code to the slow and semi reliable variant.

@Strepto
Copy link
Collaborator

Strepto commented Feb 17, 2022

Suggestion for shared algebra functions? Maybe it is time to move them in RvmSharp out of CadRevealComposer

Yeah, they could to go into RvmSharp in my mind. Most of the Math/Algebra utils have test coverage already so the quality is OK, and it should be relatively easy to move them into RvmSharp afaik.

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

Note: the PR is not tested by current build pipeline, since the feature is opt-in (either by -z|-optimize flag to RvmSharp.Exe or by passing options to RvmParser class). The benefit to CadRevealComposer is small (since CRC already do instancing on CRC's side), however it will reduce number of facetgroups considerably, that needs to go through facet group matcher. I can temporarily activate the code path in CRC to see the effect

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

Timings for HDA:
CRC conversion without optimization: 1m 06s, FacetGroup matching 7 s,
Found 11,384 unique from a total of 65,470 (82.6 %). Time: 00:00:07.0257715
CRC conversion with optimization: 1m 37s, FacetGroup matching 5s,
Found 11,382 unique from a total of 56,484 (79.8 %). Time: 00:00:05.9046869
Will run om MEL where the difference should be more substantial.

time increase is probably attributed to import analysis.

NOTE: interesting to see -2 reduction in templates. That means that facet group matcher might not manage some corner cases.

@minorai
Copy link
Collaborator Author

minorai commented Feb 17, 2022

MEL before 19m12s
Found 218,438 unique from a total of 1,644,891 (86.7 %). Time: 00:12:04.1782525
https://ws1611.statoil.net/viewLog.html?buildId=138159&buildTypeId=Melkoya_RevealMelkYaAsb&tab=buildLog&_focus=579#_state=254
MEL after 18m09s
Found 218,432 unique from a total of 1,548,117 (85.9 %). Time: 00:11:21.6529601
https://ws1611.statoil.net/viewLog.html?buildId=138314&buildTypeId=Melkoya_RevealMelkYaAsb&tab=buildLog&_focus=667

@vegasten vegasten added the prototype Prototype or proof of concept label Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prototype Prototype or proof of concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants