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

Feature: add option to call process function on a sliding window #54

Closed
frankenjoe opened this issue Jun 23, 2022 · 7 comments · Fixed by #60
Closed

Feature: add option to call process function on a sliding window #54

frankenjoe opened this issue Jun 23, 2022 · 7 comments · Fixed by #60
Labels
enhancement New feature or request

Comments

@frankenjoe
Copy link
Collaborator

frankenjoe commented Jun 23, 2022

Currently, when working with a sliding window, the user is responsible to handle the windowing. Unless in the case that a function from an external library is called that already implements a sliding window, this makes it quite inconvenient for a user to work with this feature. E.g. the following (not very efficient) code is needed to just calculate the mean over a sliding window:

def mean(signal, sampling_rate, win_dur, hop_dur):
    win_dur = int(sampling_rate * win_dur)
    hop_dur = int(sampling_rate * hop_dur)
    steps = (signal.shape[-1] - win_dur) // hop_dur
    pos = 0
    y = []
    for _ in range(steps):
        win = signal[0, pos:pos+win_dur]
        y.append(win.mean())
        pos += hop_dur
    return np.atleast_2d(y)

win_dur=1.0
hop_dur=0.5
interface = audinterface.Feature(
    'mean',
    process_func=mean,
    process_func_args={
        'win_dur': win_dur,
        'hop_dur': hop_dur,
    },
    win_dur=win_dur,
    hop_dur=hop_dur,
)

And it's also easy to mess up, e.g. the following would work, but produce wrong timestamps:

win_dur=1.0
hop_dur=0.5
interface = audinterface.Feature(
    'mean',
    process_func=mean,
    process_func_args={
        'win_dur': win_dur,
        'hop_dur': hop_dur,
    },
    win_dur=win_dur * 2,
    hop_dur=hop_dur * 2,
)

I think it would be very convenient to offer an option that the windowing is handled internally so that the user only has to provide the actual processing code, e.g.:

def mean(signal, sampling_rate):
    return signal.mean()

win_dur=1.0
hop_dur=0.5
interface = audinterface.Feature(
    'mean',
    process_func=mean,
    process_func_sliding=True,  # apply sliding window on signal and forward only single frames to processing function
    win_dur=win_dur,
    hop_dur=hop_dur,
)
@frankenjoe frankenjoe added the enhancement New feature or request label Jun 23, 2022
@hagenw
Copy link
Member

hagenw commented Jun 24, 2022

Yes, seems to make sense to provide this.

process_func_sliding=True seems to be not the optimal name as it would suggest to me that it indicates that the process function applies already a sliding window, which is not the case ;)

@frankenjoe
Copy link
Collaborator Author

process_func_sliding=True seems to be not the optimal name as it would suggest to me that it indicates that the process function applies already a sliding window, which is not the case ;)

Yes, I'm also not happy with it. Any suggestions?

@hagenw
Copy link
Member

hagenw commented Jun 24, 2022

I think we have two options for the name. It could reflect that the interface will do it for use, e.g. apply_sliding_window=False, or it could state if the process function comes with a sliding window, e.g. process_func_has_sliding_window=False.

The problem you describe in:

win_dur=1.0
hop_dur=0.5
interface = audinterface.Feature(
    'mean',
    process_func=mean,
    process_func_args={
        'win_dur': win_dur,
        'hop_dur': hop_dur,
    },
    win_dur=win_dur * 2,
    hop_dur=hop_dur * 2,
)

will not be solved by it. We could try to hand the win_dur and hop_dur silently to the process_func if given as arguments and process_func_has_sliding_window=True, but this will only work if the process function uses the same argument names, which is very unlikely. Otherwise we would need to provide some kind of mapping, which would most likely require another argument.

@frankenjoe
Copy link
Collaborator Author

apply_sliding_window=False sounds good

We could try to hand the win_dur and hop_dur silently to the process_func if given as arguments and process_func_has_sliding_window=True, but this will only work if the process function uses the same argument names, which is very unlikely.

I am not sure we have to pass win_dur and hop_dur to the processing function any longer. I will try to create a PR.

@hagenw
Copy link
Member

hagenw commented Jun 24, 2022

I am not sure we have to pass win_dur and hop_dur to the processing function any longer

I think there will be processing functions that cannot be executed without their build sliding window approach and for those we have to hand over win_dur and hop_dur. But yeah, would be nice if this is done automayically.

@frankenjoe
Copy link
Collaborator Author

frankenjoe commented Jun 24, 2022

I think there will be processing functions that cannot be executed without their build sliding window approach and for those we have to hand over win_dur and hop_dur. But yeah, would be nice if this is done automayically.

Ah that you mean! Right, when the function applies the sliding window I don't see a way we can avoid a possible mismatch.

@frankenjoe
Copy link
Collaborator Author

frankenjoe commented Jul 7, 2022

I think there will be processing functions that cannot be executed without their build sliding window approach and for those we have to hand over win_dur and hop_dur. But yeah, would be nice if this is done automayically.

In #60 it is now implemented so that the win_dur and hop_dur arguments are automatically passed on if the processing function expects them but they are not specified in process_func_args.

@hagenw hagenw closed this as completed in #60 Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants