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

Enhancements error viz #115

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Enhancements error viz #115

wants to merge 7 commits into from

Conversation

eleniv3d
Copy link
Collaborator

Still work in progress. I open it for discussion and in order to keep track of what's missing

@DamienGilliard
Copy link
Collaborator

@eleniv3d Thanks ! I took the liberty of linking the issues, so they are closed automatically when merging.

I was just looking at the per-face metric issue. Do you see a clear way to solve it ? I'm just diving in and it's not 100% clear to me how we can do that the most efficiently.

@DamienGilliard DamienGilliard linked an issue Sep 20, 2024 that may be closed by this pull request
@eleniv3d
Copy link
Collaborator Author

eleniv3d commented Sep 20, 2024

@eleniv3d Thanks ! I took the liberty of linking the issues, so they are closed automatically when merging.

I was just looking at the per-face metric issue. Do you see a clear way to solve it ? I'm just diving in and it's not 100% clear to me how we can do that the most efficiently.

@DamienGilliard
I guess one way could be to already segment the pcd per face with a similar mechanism as for the joints. Perhaps the joint segmentator could already output clusters for faces directly if it doesn't add to the calculation time. Then the pcd2mesh distance would automatically know that it is calculating for every face and will pop the corresponding message as is now the case for joints vs beams

The other way would be to specify that we want the calculation to happen per face with another toggle or ideally if we manage to make issue #116 work. but then again the distance calculation would look in the whole pcd of the joint.

@9and3
Copy link
Contributor

9and3 commented Sep 20, 2024

@DamienGilliard
I guess one way could be to already segment the pcd per face with a similar mechanism as for the joints. Perhaps the joint segmentator could already output clusters for faces directly if it doesn't add to the calculation time. Then the pcd2mesh distance would automatically know that it is calculating for every face and will pop the corresponding message as is now the case for joints vs beams

can we not use the DFFace's brep face instead of the entire brep when doing the calculation of the error @eleniv3d ? And repeat it for each DFFace. I have some doubts about having an extra clustering component just for faces.

The other way would be to specify that we want the calculation to happen per face with another toggle or ideally if we manage to make issue #116 work. but then again the distance calculation would look in the whole pcd of the joint.

This could be nice and easy the same way we did it for the VIzSettings

@DamienGilliard
Copy link
Collaborator

DamienGilliard commented Sep 20, 2024

@eleniv3d we do agree that you want a list of lists of pointclouds (and None if no pointcloud was found)? :
[[face_1_joint_1, face_2_joint_1, ...], [face_1_joint_2, face_2_joint_2, ...], ..., [face_1_joint_n, face_2_joint_n...]]
(Just double checking, I am the kind of person who messes this kind of things easily)

@eleniv3d
Copy link
Collaborator Author

@eleniv3d we do agree that you want a list of lists of pointclouds (and None if no pointcloud was found)? : [[face_1_joint_1, face_2_joint_1, ...], [face_1_joint_2, face_2_joint_2, ...], ..., [face_1_joint_n, face_2_joint_n...]] (Just double checking, I am the kind of person who messes this kind of things easily)

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment