Skip to content

I found a improved version for the ear index finder, which completely… #146

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

corbsmaster
Copy link

… avoids picking the tragus. Therefore the tolerance is used as a radius for a cylinder which is search for the face with minimal y position. In my experience it works very well for tolerances up to 3.5

… avoids picking the tragus. Therefore the tolerance is used as a radius for a cylinder which is search for the face with minimal y position. In my experience it works very well for tolerances up to 3.5
Copy link
Contributor

@f-brinkmann f-brinkmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort. Did you experience points on the tragus being picked? I like shorter code, but I think there is one difference to the old version that we should discuss:

The new code uses the face with the minimum absolute y-coordinate regardless of the distance to the y-axis. This means we could get an element that is further away from the y-axis than needed.

The old version checks every face with abs(y) < min(abs(y)) and of all those uses the face closest to the y-axis. I think this is closer to the classical Definition of the blocked ear channel position. Do you have an idea to maintain this behavior in your code?

@corbsmaster
Copy link
Author

corbsmaster commented Sep 24, 2024

I totally get your point. But yes it has in deed happened to me, that the tragus was picked with the old version of the code.
In the special case when there is a face on the tragus within min(abs(y)) + tolerance it becomes a flip of a coin if the tragus or the actual ear canal is picked. By choosing the face with minimum y value this problem is eliminated fully.

I thought of also including a penalty for higher distance from the y axis, but in the end refrained to for two reasons:
a) Is it a bug or a feature: for all the data I've seen (working with the SONICOM database), a small tolerance for the algorithm to follow the y gradient always led to picking a face closer to the visual center of the ear canal. Of course, this is database specific and could be seen as an inaccuracy of SONICOM, that the ear canal often seems to be a little in front (meaning positiv x) of the y axis. But the tolerance parameter give the posibiltity to controll this behaviour.
b) If you don't want to deviate from the y axis, setting the tolerance to the edge length, i.e. 0.5 for nicely graded meshes, is guaranteed to give at least the face next to the y axis intersection.

Let me know what you think and if you would prefer a penalty for xz-distance like so:

f_median= f.calc_center_median()  # compute face location
        xz_distance = np.sqrt(f_median.x**2 + f_median.z**2)  # compute distance to y axis
        if xz_distance < tolerance:
            if f_median.y > 0 and ear in ("Both ears", "Left ear"):  # Left
                if f_median.y < min_y[0] - alpha * xz_distance:
                    min_y[0] = f_median.y
                    left_index = f.index
            elif f_median.y < 0 and ear in ("Both ears", "Right ear"):  # Right
                if abs(f_median.y) < min_y[1] - alpha * xz_distance:
                    min_y[1] = abs(f_median.y)
                    right_index = f.index

This penalty could and should be finetuned with alpha

@f-brinkmann
Copy link
Contributor

f-brinkmann commented Sep 25, 2024

Thanks for the explanation, that makes sense to me. I think if we were to strictly follow the classical HRTF definition with the centers of the ear channes entrances being on the interaural axis (y-axis in this case) the current version of the code would be preferable. But I've also seen meshes including our own, that were a little off with respect to this definition.

I'm a little hesitant to change existing behavior if its not clearly a bug, and picking points on the tragus could potentially be avoided in the existing code if playing around with the tolerance. How about keeping the old behavior as the default but add a new parameter mode that could eiter be 'xz_distance' to use the current version or 'y_distance' to use your selection method?

@corbsmaster
Copy link
Author

Did it. I hope it doesn't lead to confusions :)
Cheers

@f-brinkmann f-brinkmann changed the base branch from master to develop September 30, 2024 08:50
@f-brinkmann
Copy link
Contributor

Thanks, that should work from my perspective. I changed the pull to go into develop and I can take care of making a new release right after merging. I might add some detail to the docstring to make the function easier to understand and ping you for review :).

The last thing that is required from my side would be to update the tests in https://github.com/Any2HRTF/Mesh2HRTF/blob/master/tests/test_assign_materials.py

They test the material assignmend on a simple ico_sphere object. If we are lucky, the results of the new mode are the same in this case and we can add a `@pytest.mark.parametrize('tolerance_mode', ['y_distance', 'xz_distance']). There is some information for running the tests in https://github.com/Any2HRTF/Mesh2HRTF/blob/master/CONTRIBUTING.rst - but I'm happy to help if you run into problems.

@f-brinkmann
Copy link
Contributor

f-brinkmann commented May 14, 2025

@corbsmaster @germknoedlspeck , did the issue come up with the SONICOM plugged meshes. I took a look at them and think that they are not well aligned. The image below shows the ear of P0001_pluged.stl and the interaural axis (y-axis) indicated by a cylinder. By definition, the y-axis should pass through the center of the (blocked) ear channel entries, which is clearly not the case here. If this is the case, I would suggest to align the meshes to the y-axis, instead of changing the code.

Screenshot 2025-05-14 at 12 09 20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants