Skip to content

Conversation

@notkaramel
Copy link
Member

@notkaramel notkaramel commented Jul 19, 2023

Changes Made

  • Issue: French TTS (espnet-tts-fr) struggles with numerical numbers #723
  • Added num2words library to help convert numbers in numeric form to text (e.g. "42" to "quarante deux")
  • Added debugging logs indicating cases of numeric conversion: ("segment" as a part of the input string that's separated by a space)
    • Case 0: segment is numeric or float number (e.g., "2012", "23.12", "2021."). This includes case where a number is at the end of a sentence with "." as a part of the segment.
    • Case 1: segment has ",". This is due to multilang-support tranlated to Metropolitan French where they have a different notation system.
    • Case 2: segment is a range, containing a hyphen '-'. This is often seen in charts with age or year range. It also requires extra syntax to be added. E.g., "34-59" would be "de trente quatre à cinquante neuf".
    • NOTE: This might conflict with words in French that has hyphen, like "gratte-ciel", "arc-en-ciel", or names like "Lionel-Groulx".

Testings


Please ensure you've followed the checklist and provide all the required information before requesting a review.
If you do not have everything applicable to your PR, it will not be reviewed!
If you don't know what something is or if it applies to you, ask!

Don't delete below this line.


Required Information

  • I referenced the issue addressed in this PR.
  • I described the changes made and how these address the issue.
  • I described how I tested these changes.

Coding/Commit Requirements

  • I followed applicable coding standards (e.g., PEP8)
  • I have not committed any models or other large files.

New Component Checklist (mandatory for new microservices)

  • I added an entry to docker-compose.yml and build.yml.
  • I created A CI workflow under .github/workflows.

@notkaramel notkaramel assigned JRegimbal and unassigned notkaramel Jul 24, 2023
Comment on lines 44 to 49
def isfloat(num):
try:
float(num)
return True
except ValueError:
return False
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this because there's (supposedly) no built-in function in Python to check if a number is a float.

Copy link
Collaborator

@JRegimbal JRegimbal left a comment

Choose a reason for hiding this comment

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

Looking great!

logger.debug(segment.split())
for s in segment.split():
try:
logger.debug(f'Performing on: "{s}"')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to get very chatty very quickly...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not merging right now, but do you anticipate this being useful going forward? I'm a little worried about the slim possibility of this leaking some personal info.

Copy link
Member Author

Choose a reason for hiding this comment

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

I find this helpful to debug what component the TTS couldn't convert to numbers sometimes, but it can definitely be case specific (log in the except statement) rather than logging out everything.

You might be right... In that case, I would have to change the logs of multilang-support as well because it has been logging the whole segment out everytime, both in English and French 🫤

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, yeah please do that too then!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done that!

@JRegimbal JRegimbal assigned notkaramel and unassigned JRegimbal Jul 25, 2023
@notkaramel
Copy link
Member Author

@JRegimbal I just reformat the processSegment so that it would let normal text through + added edge case handling for hyphen cases: it can now handle number range and also words/names like "gratte-ciel" (skyscrapper), "Lionel-Groulx", etc., without trying to convert them to number :)

@notkaramel notkaramel assigned JRegimbal and unassigned notkaramel Jul 26, 2023
s = num2words(float(tempns), lang='fr')
elif "-" in s:
logger.debug("case 3: -")
logger.debug("Case: has a - as separator")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this would likely fail for cases where two separators are used. E.g., "39,5-40,0". We could probably do something with regexps with the - separator as well. Get the fields we need excluding text with ([\d,.]+)-([\d,.]+), replace with de $1 à $2 and then apply another find and replace on $1 and $2 to replace occurrences of , with .. Does this make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it does. I will try them out.

s = num2words(s, lang='fr')
elif "," in s:
logger.debug(f"Case: '{s}' has , as separator")
logger.debug("Case: has a , as separator")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, does this still get run for text uses of ,?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could also catch with a regexp find replace ((\d+),(\d+) to $1.$2).

Copy link
Member Author

Choose a reason for hiding this comment

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

It catches text with ,, but it won't perform any changes since on the next line, it couldn't cast the text to float.

I will look into regex and string processing more, thanks for the suggestion! I genuinely forgot about regex and went straight for the simpliest implementation.

@notkaramel
Copy link
Member Author

@notkaramel notkaramel assigned JRegimbal and unassigned notkaramel Aug 4, 2023
Copy link
Collaborator

@JRegimbal JRegimbal left a comment

Choose a reason for hiding this comment

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

Looks good!

@JRegimbal
Copy link
Collaborator

@notkaramel you can make a new issue, but I'd put it in backlog for right now and bring it up tomorrow if you're still looking for something to do. There may be a higher priority issue.

@JRegimbal JRegimbal merged commit a7c79ad into main Aug 7, 2023
@JRegimbal JRegimbal deleted the french-tts-vs-numbers branch August 7, 2023 14:59
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 this pull request may close these issues.

French TTS (espnet-tts-fr) struggles with numerical numbers

3 participants