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

#1203 Update compute_supervisions_frame_mask() to skip negative end indices #1204

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

Conversation

ArthLeu
Copy link

@ArthLeu ArthLeu commented Nov 4, 2023

When flipping mask digits to 1.0, negative indices should never be used.

…end indices

When flipping mask digits to 1.0, negative indices should never be used.
@ArthLeu ArthLeu changed the title #1203 Update supervision_frame_masks() to skip negative end indices #1203 Update compute_supervisions_frame_mask() to skip negative end indices Nov 4, 2023
@pzelasko
Copy link
Collaborator

pzelasko commented Nov 5, 2023

Right. I triggered the tests, if they're green, LGTM.

@pzelasko pzelasko added this to the v1.18 milestone Nov 5, 2023
@ArthLeu
Copy link
Author

ArthLeu commented Nov 8, 2023

It wants some newlines. Can this automatically apply the changes or do I need to manually change them?

@desh2608
Copy link
Collaborator

desh2608 commented Nov 8, 2023

You would need to run black formatting. See development instructions here: https://github.com/lhotse-speech/lhotse#development-installation

@pzelasko
Copy link
Collaborator

Please follow these instructions, pre-commit will auto-apply relevant changes, but you'll need to commit them yourself.

https://github.com/lhotse-speech/lhotse#development-installation

@ArthLeu
Copy link
Author

ArthLeu commented Dec 22, 2023

Sorry I was busy with something else. I upgraded to Lhotse 1.18, and seems like the Cut.supervision_frame_masks() now uses relative time to determine mask region.

So this means when we need to use_alignment_if_exists, the alignment timestamps should also be relative? I tried absolute alignment timestamps, which worked with 1.17, and does not work with 1.18.

Want to confirm this before making any further change.

Probably related to #1193

@desh2608
Copy link
Collaborator

Sorry I was busy with something else. I upgraded to Lhotse 1.18, and seems like the Cut.supervision_frame_masks() now uses relative time to determine mask region.

So this means when we need to use_alignment_if_exists, the alignment timestamps should also be relative? I tried absolute alignment timestamps, which worked with 1.17, and does not work with 1.18.

Want to confirm this before making any further change.

Probably related to #1193

That PR is a bug fix, not a change in behavior. Alignments were always supposed to have absolute start times (w.r.t. the recording), whereas supervisions are supposed to be relative (w.r.t the cut).

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.

3 participants