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

[Bug] Punctuation restoration works incorrect #3333

Closed
Squire-tomsk opened this issue Nov 29, 2023 · 1 comment · Fixed by #3336
Closed

[Bug] Punctuation restoration works incorrect #3333

Squire-tomsk opened this issue Nov 29, 2023 · 1 comment · Fixed by #3336
Labels
bug Something isn't working

Comments

@Squire-tomsk
Copy link

Describe the bug

Punctuation restoration works incorrect.

To Reproduce

from TTS.tts.utils.text.punctuation import Punctuation, _PUNC_IDX, PuncPosition

# original text "...i think i understand."
punctuator = Punctuation()
text = ['', 'i think i understand']
punctuation = [_PUNC_IDX(punc='...', position=PuncPosition.BEGIN), _PUNC_IDX(punc='.', position=PuncPosition.END)]
punctuator.restore(text, punctuation)

# result ["....", "i think i understand"]

Expected behavior

result: ["...i think i understand."]

Logs

No response

Environment

{
    "CUDA": {
        "GPU": [
            "NVIDIA RTX A6000"
        ],
        "available": true,
        "version": "11.8"
    },
    "Packages": {
        "PyTorch_debug": false,
        "PyTorch_version": "2.0.0+cu118",
        "TTS": "0.16.6",
        "numpy": "1.22.0"
    },
    "System": {
        "OS": "Linux",
        "architecture": [
            "64bit",
            ""
        ],
        "processor": "x86_64",
        "python": "3.10.12",
        "version": "#170-Ubuntu SMP Fri Jun 16 13:43:31 UTC 2023"
    }
}

Additional context

No response
@Squire-tomsk Squire-tomsk added the bug Something isn't working label Nov 29, 2023
eginhard added a commit to idiap/coqui-ai-TTS that referenced this issue Nov 29, 2023
@eginhard
fix(punctuation): correctly handle initial punctuation
45200d1 
Stripping and restoring initial punctuation didn't work correctly because the
string-splitting caused an additional empty string to be inserted in the text
list (because `".A".split(".")` => `["", "A"]`). Now, an initial empty string is
skipped and relevant test cases are added.

Fixes coqui-ai#3333
@eginhard eginhard mentioned this issue Nov 29, 2023
Merged
@eginhard
Copy link
Contributor

Seems like these methods didn't correctly support initial punctuation - I opened #3336 to fix.

erogol pushed a commit that referenced this issue Nov 30, 2023
@eginhard
fix: correctly strip/restore initial punctuation (#3336)
39321d0 
* refactor(punctuation): remove orphan code for handling lone punctuation

The case of lone punctuation is already handled at the top of restore(). The
removed if statement would never be called and would in fact raise an
AttributeError because the _punc_index named tuple doesn't have the attribute
`mark`.

* refactor(punctuation): remove unused argument

* fix(punctuation): correctly handle initial punctuation

Stripping and restoring initial punctuation didn't work correctly because the
string-splitting caused an additional empty string to be inserted in the text
list (because `".A".split(".")` => `["", "A"]`). Now, an initial empty string is
skipped and relevant test cases are added.

Fixes #3333
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: correctly strip/restore initial punctuation idiap/coqui-ai-TTS
2 participants
@eginhard @Squire-tomsk