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.

Sign up for GitHub

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

Jump to bottom

Unnest Audio Feature Preprocessing Config #2242

Merged
merged 7 commits into from
Jul 9, 2022
Merged

Unnest Audio Feature Preprocessing Config #2242

merged 7 commits into from
Jul 9, 2022

Conversation

connor-mccorm
Copy link
Contributor

@connor-mccorm connor-mccorm commented Jul 7, 2022 •
edited
Loading

Un-nesting the Audio Feature Preprocessing Parameter config so that it is more consistent and is easier to deal with for the frontend. Currently, the audio feature config looks like this:

            "audio": {
                "audio_file_length_limit_in_s": 7.5,
                "missing_value_strategy": "backfill",
                "in_memory": True,
                "padding_value": 0,
                "norm": None,
                "audio_feature": {
                    "type": "fbank",
                    "window_length_in_s": 0.04,
                    "window_shift_in_s": 0.02,
                    "num_filter_bands": 80,
                },

With the un-nested audio feature, it will look like this:

"audio": {
                "audio_file_length_limit_in_s": 7.5,
                "missing_value_strategy": "backfill",
                "in_memory": True,
                "padding_value": 0,
                "norm": None,
                "type": "fbank",
                "window_length_in_s": 0.04,
                "window_shift_in_s": 0.02,
                "num_fft_points": None,
                "window_type": "hamming",
                "num_filter_bands": 80,
            },

@github-actions
Copy link

github-actions bot commented Jul 7, 2022 •
edited
Loading

Unit Test Results

       6 files  +    1         6 suites  +1   2h 31m 41s ⏱️ + 46m 37s
2 920 tests ±    0  2 874 ✔️ +    1    46 💤 ±  0  0 ❌  - 1 
8 760 runs  +169  8 618 ✔️ +147  142 💤 +23  0 ❌  - 1 

Results for commit 2081105. ± Comparison against base commit 8cc5b44.

♻️ This comment has been updated with latest results.

@connor-mccorm connor-mccorm requested review from justinxzhao, geoffreyangus and ksbrar and removed request for geoffreyangus July 8, 2022 18:53
@connor-mccorm connor-mccorm changed the title Audio feature nest Unnest Audio Feature Preprocessing Config Jul 8, 2022
@ksbrar
Copy link
Collaborator

ksbrar commented Jul 8, 2022

@connor-mccorm for completeness do you mind adding an example of what the unnesting looks like to the description? (e.g. copy paste from the tests)

ksbrar
ksbrar approved these changes Jul 8, 2022
View reviewed changes
Copy link
Collaborator

@ksbrar ksbrar left a comment

Choose a reason for hiding this comment

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

Nice! LGTM, just a few Qs.

ludwig/features/audio_feature.py Show resolved Hide resolved
ludwig/schema/preprocessing.py Show resolved Hide resolved
ludwig/constants.py Show resolved Hide resolved
@connor-mccorm
Copy link
Contributor Author

Added description @ksbrar

@connor-mccorm connor-mccorm merged commit 2961eb9 into master Jul 9, 2022
@connor-mccorm connor-mccorm deleted the audio_feature_nest branch July 9, 2022 02:46
abidwael pushed a commit that referenced this pull request Jul 11, 2022
@connor-mccorm @pre-commit-ci
Unnest Audio Feature Preprocessing Config (#2242)
5619006 
* Unnest Audio Feature

* Added schema refactoring

* Flake 8

* Fixed torchscript issue

* Removed audio_feature schema param, whoops. Addressed feedback

* Fixed ray audio test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksbrar ksbrar ksbrar approved these changes

@justinxzhao justinxzhao justinxzhao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@connor-mccorm @ksbrar @justinxzhao