Request for PR review: Add support for Thai language

I have created the following PR to add support for Thai language.
I am in the process of creating a dataset to train the model but would love a PR review of the code first to make sure I am on the right track.
Thank you!
#117

I planed to work on training a Thai TTS model using MeloTTS too.

Hi - Thanks for the contribution. We would suggest you first train on the Thai dataset to see if the code works. We haven't had any attempt to train on Thai

@Zengyi-Qin Sounds good, will report back once I have proper training results.

Thank you @tchayintr, if you have any recommendations for Thai audio datasets, I would greatly appreciate it!

@jadechip
Sure! There are several datasets such as TSync2, Lotus, etc. You can check several of them here: https://github.com/korakot/corpus/releases/tag/v1.0 with documentation at https://lexitron.nectec.or.th/KM_HL5001/file_HL5001/Document/krrn_14518.pdf.
There are also Thai dialects available at https://github.com/SLSCU/thai-dialect-corpus.
However, I recommend collecting clear voice clips and crafting their transcriptions with ASR tools like WhisperX. This way, you can generate a lot of samples, but you may need to fine-tune it for the Thai language 😄.
I am reviewing your commits too. They mostly look great 🎆 , but I found some points that need to be clarified.
I will clarify and let you know if there is a point that may need to be adjusted in terms of Thai linguistic knowledge.

Nevermind, I was able to pinpoint the issue, I didn't realize you needed to add the language code here as well:

I seems like it is training correctly now although I am still getting some warnings/exceptions:
Using noise scaled MAS for VITS2
Using duration discriminator for VITS2
(torch.Size([219, 192]), torch.Size([360, 192]))
(torch.Size([16, 192]), torch.Size([17, 192]))
(torch.Size([10, 192]), torch.Size([9, 192]))
(torch.Size([256, 256]), torch.Size([1, 256]))
list index out of range
  0%|                                                                                | 0/23601 [00:00<?, ?it/s][W reducer.cpp:1298] Warning: find_unused_parameters=True was specified in DDP constructor, but did not find any unused parameters in the forward pass. This flag results in an extra traversal of the autograd graph every iteration,  which can adversely affect performance. If your model indeed never has any unused parameters in the forward pass, consider turning this flag off. Note that this warning may be a false positive if your model has flow control causing later iterations to have unused parameters. (function operator())
/home/zeus/miniconda3/envs/cloudspace/lib/python3.10/site-packages/torch/autograd/__init__.py:197: UserWarning: Grad strides do not match bucket view strides. This may indicate grad was not created according to the gradient layout contract, or that the param's strides changed since DDP was constructed.  This is not an error, but may impair performance.
grad.sizes() = [1, 9, 96], strides() = [99168, 96, 1]
bucket_view.sizes() = [1, 9, 96], strides() = [864, 96, 1] (Triggered internally at ../torch/csrc/distributed/c10d/reducer.cpp:325.)
  Variable._execution_engine.run_backward(  # Calls into the C++ engine to run the backward pass
Evaluating ...
Evauate done
  0%|▍                                                                                | 74/23601 [03:24<11:00:36,  1.68s/it]min value is  tensor(-1.1265)

hello @jadechip
let me know if its working
i'm training for indonesia and malay language
changing phonem and bert also
after 10 epoch the model doesnt produce any good word, only some noise , some random vowel

hello @jadechip @acul3
It seems there might be an issue with the training process. According to the current code, if your symbol size is not equal to the original 219, a new size will be used to initialize the TextEncoder. This means that you are not utilizing the base model, but rather retraining it. Based on my previous tests, this could lead to strange results where the model fails to properly generate text.
Solution: Similar to adding a new vocabulary to BERT, you should modify the loading process of the model. https://huggingface.co/transformers/v2.11.0/_modules/transformers/modeling_utils.html (_get_resized_embeddings function)

Thank you @jeremy110.
If I understand correctly in melo/models.py, we should first initialize the TextEncoder with the original 219, in order to use the retrained weights, like this:
// models.py
        self.enc_p = TextEncoder(
            219,  # Initialize with the original symbol size
            inter_channels,
            hidden_channels,
            filter_channels,
            n_heads,
            n_layers,
            kernel_size,
            p_dropout,
            gin_channels=self.enc_gin_channels,
            num_languages=num_languages,
            num_tones=num_tones,
        )

...then right after add a check if the n_vocab (len(symbols)) has a different size, and if so update the     self.enc_p.embed_tokens with the resized embeddings?
if n_vocab != 219:
    old_embeddings = self.enc_p.embed_tokens
    new_num_tokens = n_vocab
    self.enc_p.embed_tokens = _get_resized_embeddings(old_embeddings, new_num_tokens)

Does that look correct to you?

hello~ @jadechip
Yes, it looks fine as it is.
However, in symbols.py, you'll need to make some modifications. If you place your new symbol inside the sorted list and then use the method above, it may result in some symbols having weights that don't match up with the original model. So, I suggest you do it like this.
# combine all symbols
normal_symbols = sorted(set(zh_symbols + ja_symbols + en_symbols + kr_symbols + es_symbols + fr_symbols + de_symbols + ru_symbols))
symbols = [pad] + normal_symbols + pu_symbols + new_symbols # add new symbols here

I see, thank you for the heads up @jeremy110 🙏
I've updated my code to reflect your suggestion, now I have.
# combine all symbols
normal_symbols = sorted(set(zh_symbols + ja_symbols + en_symbols + kr_symbols + es_symbols + fr_symbols + de_symbols + ru_symbols))
symbols = [pad] + normal_symbols + pu_symbols + th_symbols
sil_phonemes_ids = [symbols.index(i) for i in pu_symbols]

# combine all tones
num_tones = num_zh_tones + num_ja_tones + num_en_tones + num_kr_tones + num_es_tones + num_fr_tones + num_de_tones + num_ru_tones + num_th_tones

# language maps
language_id_map = {"ZH": 0, "JP": 1, "EN": 2, "ZH_MIX_EN": 3, 'KR': 4, 'ES': 5, 'SP': 5, 'FR': 6, 'TH': 7}
num_languages = len(language_id_map.keys())

I'll try running a new training job to evaluate performance with these changes.

thanks @jadechip and @jeremy110
i'll try it to my environment also,see if works

Ok, I was able to run a training job for around 9k steps yesterday. I tried running inference using the new checkpoint, but it seems to produce unintelligible sounds. I think the learning rate looks ok though? ...so I will try ramping up the batch size and training for longer on multiple GPUs and report back with my results 🤞
For reference here is my current config and Tensorboard metrics.
{
  "train": {
    "log_interval": 200,
    "eval_interval": 1000,
    "seed": 52,
    "epochs": 10000,
    "learning_rate": 0.0003,
    "betas": [
      0.8,
      0.99
    ],
    "eps": 1e-09,
    "batch_size": 16,
    "fp16_run": false,
    "lr_decay": 0.999875,
    "segment_size": 16384,
    "init_lr_ratio": 1,
    "warmup_epochs": 0,
    "c_mel": 45,
    "c_kl": 1.0,
    "skip_optimizer": true
  },
  "data": {
    "training_files": "../Data/locutor/train.list",
    "validation_files": "../Data/locutor/val.list",
    "max_wav_value": 32768.0,
    "sampling_rate": 44100,
    "filter_length": 2048,
    "hop_length": 512,
    "win_length": 2048,
    "n_mel_channels": 128,
    "mel_fmin": 0.0,
    "mel_fmax": null,
    "add_blank": true,
    "n_speakers": 1,
    "cleaned_text": true,
    "spk2id": {
      "locutor": 0
    }
  },
  "model": {
    "use_spk_conditioned_encoder": true,
    "use_noise_scaled_mas": true,
    "use_mel_posterior_encoder": false,
    "use_duration_discriminator": true,
    "inter_channels": 192,
    "hidden_channels": 192,
    "filter_channels": 768,
    "n_heads": 2,
    "n_layers": 6,
    "n_layers_trans_flow": 3,
    "kernel_size": 3,
    "p_dropout": 0.1,
    "resblock": "1",
    "resblock_kernel_sizes": [
      3,
      7,
      11
    ],
    "resblock_dilation_sizes": [
      [
        1,
        3,
        5
      ],
      [
        1,
        3,
        5
      ],
      [
        1,
        3,
        5
      ]
    ],
    "upsample_rates": [
      8,
      8,
      2,
      2,
      2
    ],
    "upsample_initial_channel": 512,
    "upsample_kernel_sizes": [
      16,
      16,
      8,
      2,
      2
    ],
    "n_layers_q": 3,
    "use_spectral_norm": false,
    "gin_channels": 256
  },
  "num_languages": 1,
  "num_tones": 16,
  "symbols": [
...

btw I am currently training on a subset of Thai commonvoice 13, converted to .wav with a sample rate of 48 kHz.

hello~ @jadechip
My config is basically the same as yours, except my batch size is 6. Perhaps you can increase your learning rate to 9e-4 and see how it performs. Also, I've added a constraint to the clip_grad_value in the code.
grad_norm_d = commons.clip_grad_value_(net_d.parameters(), 200)
grad_norm_g = commons.clip_grad_value_(net_g.parameters(), 500)

Finally, I'm attaching my tensorboard for reference. (https://drive.google.com/drive/folders/1xPNURmWsmJqwEDHVM8ZsK6CAbuv65ipI?usp=sharing)
Additionally, if the silence before and after your audio files is shorter, your g/dur will converge to a smaller value, which will also affect the length of the silence before and after the inference.
I'm not sure if the Thai CommonVoice 13 dataset is suitable for training. Also, there's no need to specifically convert it to 48kHz. I remember that the code will resample it. I think you can start by testing whether it can be trained with 10 hours of data from one person.
I hope this is helpful for you.

Thank you for you sharing! Your advice has been super helpful @jeremy110 🙏

Hmm trained for longer with different hyperparameters but so far the results are not much better, something might be wrong with my code.

yeah me too
longer training,,the voice is clearer and similar, but cant pronounce a single word
maybe phenomizer problem ,idk

hello @jadechip @acul3
I'd like to confirm something. Are all your tones set to 0?
Because I made a similar mistake before where I treated tones like ˧ ˦ as phones, but they should correspond to tokens. Here's an example of what I did before.
#error
phones: ['_', 'k', 'e', 'ʔ', '˧', 'p', 'i', 'a', 'ʔ', '˧', 'ʦ', 'ʰ', 'i', 'n', '˦', '˦', 'k', 'e', '˦', '˦', ',', 'l', 'e', '˥', '˧', 's', 'ɔ', '˨', '˩', 'g', 'u', 'a', 'n', '˩', '˧', 'ʦ', 'a', 'i', '˧', '˧', '.', '_']
tones: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
word2ph: [1, 4, 5, 6, 4, 1, 4, 4, 6, 5, 1, 1]

#correct
phones: 28 ['_', 'k', 'e', 'ʔ', 'p', 'i', 'a', 'ʔ', 'ʦ', 'ʰ', 'i', 'n', 'k', 'e', ',', 'l', 'e', 's', 'ɔ', 'g', 'u', 'a', 'n', 'ʦ', 'a', 'i', '.', '_']
tones: [0, 4, 4, 4, 4, 4, 4, 4, 1, 1, 1, 1, 1, 1, 0, 2, 2, 3, 3, 5, 5, 5, 5, 7, 7, 7, 0, 0]
[word2ph: [1, 3, 4, 4, 2, 1, 2, 2, 4, 3, 1, 1]

@jeremy110 yes all my tone are set to 0
now wondering how can i fix this

hello~ @acul3 @jadechip
Sorry, I spent some time looking at that, but since I can't read Thai, I did some online research. I wanted to ask about the symbols from line 266 to 339 in the th_symbols . Are those symbols not IPA?
Also, I looked at the Wiktionary file and found several symbols that seem to represent tones: ˧, ˨˩, ˦˥, ˩˦, and ˥˩. It looks like there are five tones. So, you need to convert these symbols into tones and then add the corresponding number of tones to the 'tones' list based on the number of phones in your phone list.
But I'm confused about lines 5908 to 5910. Which one is correct?

@jeremy110 you are absolutely right. My code was outputting zeroes for the tones list.
I've pushed some changes to the g2p function which hopefully addresses this:
def g2p(norm_text):
    tokenized = tokenizer.tokenize(norm_text)
    phs = []
    word2ph = []
    current_word = []
    current_phonemes = []

    for token in tokenized:
        if token.startswith("▁"):  # Start of a new word
            if current_word:
                word_phonemes = " ".join(current_phonemes)
                phs.extend(word_phonemes.split())
                word2ph.append(len(current_phonemes))
                current_word = []
                current_phonemes = []
            current_word.append(token.replace("▁", ""))
        else:
            current_word.append(token)

        if token in punctuation or token in pu_symbols:
            phs.append(token)
            word2ph.append(1)
        else:
            phonemes = thai_text_to_phonemes(token.replace("▁", ""))
            current_phonemes.extend(phonemes.split())

    if current_word:
        word_phonemes = " ".join(current_phonemes)
        phs.extend(word_phonemes.split())
        word2ph.append(len(current_phonemes))

    # Distribute phonemes to match the number of tokens
    distributed_word2ph = []
    for i, group in enumerate(tokenized):
        if group.startswith("▁"):
            group = group.replace("▁", "")
        if group in punctuation or group in pu_symbols:
            distributed_word2ph.append(1)
        else:
            phonemes = thai_text_to_phonemes(group)
            distributed_word2ph.append(len(phonemes.split()))

    tone_markers = ['˥', '˦', '˧', '˨', '˩']
    phones = ["_"] + [re.sub(f'[{"".join(tone_markers)}]', '', p) for p in phs] + ["_"]  # Remove tone markers from phones
    tones = extract_tones(phs)  # Extract tones from the original phs list
    word2ph = [1] + distributed_word2ph + [1]

    assert len(word2ph) == len(tokenized) + 2

    return phones, tones, word2ph

def extract_tones(phones):
    tones = []
    tone_map = {
        "˥": 5,  # High tone
        "˦": 4,  # Rising tone
        "˧": 3,  # Mid tone
        "˨": 2,  # Falling tone
        "˩": 1,  # Low tone
    }

    for phone in phones:
        tone = 0
        for marker, value in tone_map.items():
            if marker in phone:
                tone = value
                break
        tones.append(tone)

    return tones

TLDR;

it now removes the tone markers from the phonemes in phs using a regular expression and stores the result in the phones list, adding start and end markers ("_").
It then extracts the tones from the original phs list using the extract_tones function and stores them in the tones list.
It constructs the final word2ph list by adding start and end markers (1) to the distributed_word2ph list and finally, it returns the phones, tones, and word2ph lists.

...I've also updated the test following test case:
def test_g2p():
    text = "ฉันรักเมืองไทย"
    normalized_text = text_normalize(text)
    phones, tones, word2ph = g2p(normalized_text)
    assert phones == ['_', 't͡ɕʰ', 'a', 'n', '', 'r', 'a', 'k̚', '', 'm', 'ɯa̯', 'ŋ', '', 'tʰ', 'aj', '', '.', 'j', 'a', '', '.', '_']
    assert tones == [0, 0, 0, 4, 0, 0, 0, 5, 0, 0, 0, 3, 0, 0, 3, 0, 0, 0, 5, 0]
    assert word2ph == [1, 0, 8, 12, 1]

I think this output makes sense as the output is now similar to yours.
The phones list contains the phonemes corresponding to the input text, excluding the tone markers.
The mapping of tone markers to numeric values seems accurate (4 for ˩˩˦, 5 for ˦˥, 3 for ˧).
The word2ph list represents the number of phonemes for each word in the tokenized input. The values correspond to the number of phonemes for each word:
1: Start-of-sequence token
0: No phonemes for the first token (likely punctuation or special symbol)
8: Number of phonemes for the second token ("ฉันรัก")
12: Number of phonemes for the third token ("เมืองไทย")
1: End-of-sequence token

About the Thai symbols, the characters from line 266 to 339 are the characters of the Thai alphabet, including numbers.
The remaining lines (340 - 406) were characters that I copied from the Wiktionary file (which I got from here https://github.com/PyThaiNLP/thai-g2p-wiktionary-corpus/tree/main), I am not sure if I should include them in this file (symbols.py) but if I remember correctly I was getting an error if I didn't include them.

About lines 5908 to 5910 in the Wiktionary file, that is a good question. I am not sure which one is correct to be honest 🤔
Maybe I should try looking for a different Grapheme to Phoneme dictionary...

About lines 5908 to 5910 in the Wiktionary file, that is a good question. I am not sure which one is correct to be honest 🤔 Maybe I should try looking for a different Grapheme to Phoneme dictionary...

I appreciate your hard work. 🥇
One of my concerns is that most Thai G2P tools are either rule-based or seq2seq, and their phoneme formats vary (e.g., haas, IPA, etc.):

https://github.com/wannaphong/thai-grapheme-to-phoneme
https://github.com/nozomiyamada/thaig2p
https://github.com/wannaphong/thai-g2p
https://www.thaicorpus.net/g2p

In case, you missed some of them. 😄
While rule-based tools offer more precise conversions, they may not always provide results for some graphemes. Seq2seq tools, on the other hand, offer more flexible conversions, but their CER or PER is still considered high, IMO.
Of course, these factors can reduce the smoothness in TTS.
I am concerned about the current state of Thai G2P and am trying to survey how we can address the challenges with Thai G2P.

@jeremy110 you are absolutely right. My code was outputting zeroes for the tones list. I've pushed some changes to the g2p function which hopefully addresses this:
def g2p(norm_text):
    tokenized = tokenizer.tokenize(norm_text)
    phs = []
    word2ph = []
    current_word = []
    current_phonemes = []

    for token in tokenized:
        if token.startswith("▁"):  # Start of a new word
            if current_word:
                word_phonemes = " ".join(current_phonemes)
                phs.extend(word_phonemes.split())
                word2ph.append(len(current_phonemes))
                current_word = []
                current_phonemes = []
            current_word.append(token.replace("▁", ""))
        else:
            current_word.append(token)

        if token in punctuation or token in pu_symbols:
            phs.append(token)
            word2ph.append(1)
        else:
            phonemes = thai_text_to_phonemes(token.replace("▁", ""))
            current_phonemes.extend(phonemes.split())

    if current_word:
        word_phonemes = " ".join(current_phonemes)
        phs.extend(word_phonemes.split())
        word2ph.append(len(current_phonemes))

    # Distribute phonemes to match the number of tokens
    distributed_word2ph = []
    for i, group in enumerate(tokenized):
        if group.startswith("▁"):
            group = group.replace("▁", "")
        if group in punctuation or group in pu_symbols:
            distributed_word2ph.append(1)
        else:
            phonemes = thai_text_to_phonemes(group)
            distributed_word2ph.append(len(phonemes.split()))

    tone_markers = ['˥', '˦', '˧', '˨', '˩']
    phones = ["_"] + [re.sub(f'[{"".join(tone_markers)}]', '', p) for p in phs] + ["_"]  # Remove tone markers from phones
    tones = extract_tones(phs)  # Extract tones from the original phs list
    word2ph = [1] + distributed_word2ph + [1]

    assert len(word2ph) == len(tokenized) + 2

    return phones, tones, word2ph

def extract_tones(phones):
    tones = []
    tone_map = {
        "˥": 5,  # High tone
        "˦": 4,  # Rising tone
        "˧": 3,  # Mid tone
        "˨": 2,  # Falling tone
        "˩": 1,  # Low tone
    }

    for phone in phones:
        tone = 0
        for marker, value in tone_map.items():
            if marker in phone:
                tone = value
                break
        tones.append(tone)

    return tones

TLDR;

it now removes the tone markers from the phonemes in phs using a regular expression and stores the result in the phones list, adding start and end markers ("_").
It then extracts the tones from the original phs list using the extract_tones function and stores them in the tones list.
It constructs the final word2ph list by adding start and end markers (1) to the distributed_word2ph list and finally, it returns the phones, tones, and word2ph lists.

...I've also updated the test following test case:
def test_g2p():
    text = "ฉันรักเมืองไทย"
    normalized_text = text_normalize(text)
    phones, tones, word2ph = g2p(normalized_text)
    assert phones == ['_', 't͡ɕʰ', 'a', 'n', '', 'r', 'a', 'k̚', '', 'm', 'ɯa̯', 'ŋ', '', 'tʰ', 'aj', '', '.', 'j', 'a', '', '.', '_']
    assert tones == [0, 0, 0, 4, 0, 0, 0, 5, 0, 0, 0, 3, 0, 0, 3, 0, 0, 0, 5, 0]
    assert word2ph == [1, 0, 8, 12, 1]

I think this output makes sense as the output is now similar to yours.
The phones list contains the phonemes corresponding to the input text, excluding the tone markers. The mapping of tone markers to numeric values seems accurate (4 for ˩˩˦, 5 for ˦˥, 3 for ˧).
The word2ph list represents the number of phonemes for each word in the tokenized input. The values correspond to the number of phonemes for each word:
1: Start-of-sequence token
0: No phonemes for the first token (likely punctuation or special symbol)
8: Number of phonemes for the second token ("ฉันรัก")
12: Number of phonemes for the third token ("เมืองไทย")
1: End-of-sequence token

text: 禮          數
ipa: l e ˥ ˧      s ɔ ˨˩
phones: ['_', 'l', 'e',     's', 'ɔ', '_']
tones: [0, 2, 2,       3, 3, 0]
word2ph: [1, 2,      2, 1]

Perhaps I misled you a bit. Let me clarify using an example.
For '˥ ˧' in my case, it corresponds to 2. Then, with two phones, 'l' and 'e', so the tones correspond to two 2.
For '˩' in my case, it corresponds to 3. Then, with two phones, 'l' and 'e', so the tones correspond to two 3.

About lines 5908 to 5910 in the Wiktionary file, that is a good question. I am not sure which one is correct to be honest 🤔 Maybe I should try looking for a different Grapheme to Phoneme dictionary...

I appreciate your hard work. 🥇
One of my concerns is that most Thai G2P tools are either rule-based or seq2seq, and their phoneme formats vary (e.g., haas, IPA, etc.):

https://github.com/wannaphong/thai-grapheme-to-phoneme
https://github.com/nozomiyamada/thaig2p
https://github.com/wannaphong/thai-g2p
https://www.thaicorpus.net/g2p

In case, you missed some of them. 😄
While rule-based tools offer more precise conversions, they may not always provide results for some graphemes. Seq2seq tools, on the other hand, offer more flexible conversions, but their CER or PER is still considered high, IMO. Of course, these factors can reduce the smoothness in TTS.
I am concerned about the current state of Thai G2P and am trying to survey how we can address the challenges with Thai G2P.

Because I don't know Thai at all, I can't help with the g2p part.
sorry

@jeremy110 Don't worry, this is not your fault at all!
We are here to discuss and find a solution.
I will keep you updated if I got something. @jadechip @jeremy110

This one is my implementation for the @jeremy110 's tones. I also trying to change @jadechip 's phonemizer which sometime return the characters not phonemes.
code
But, the problem now is the phonemizer is not that good. It's usually give  repeated phonemes for some reason.

@jeremy110 if I understand correctly, I am extracting the tones from the original phs list and assigning them to the phones in a one-to-one manner, but you are saying a single tone marker should be assigned to multiple phones based on the number of phones associated with that tone marker?
I've pushed some code that tries to solve this issue and I've added a new test cases. However I am still experiencing some strange behavior and I'm afraid I am reaching the limits of my knowledge of the Thai language 😔

@jadechip
I spent some time finding a few words from the Wiktionary file to use as examples because I noticed that the processing of Thai is different from mine, so let me give another example.
กง	k o ŋ ˧    -> 3 phones + 1 tone
ล้อ	l ɔː ˦˥        -> 2 phones + 1 tone
กงล้อ	k o ŋ ˧ . l ɔː ˦˥

suppose tone map: ˧ -> 2,  ˦˥ -> 3
text: กงล้อ -> กง   ล้อ
phones: [ '_', 'k', 'o', 'ŋ',    'l', 'ɔː', '_']
tones: [ 1, 2, 2, 2,      3, 3, 1]  # 2 copied three times (3 phones), 3 copied twice (2 phones)

@jadechip I spent some time finding a few words from the Wiktionary file to use as examples because I noticed that the processing of Thai is different from mine, so let me give another example.
กง	k o ŋ ˧    -> 3 phones + 1 tone
ล้อ	l ɔː ˦˥        -> 2 phones + 1 tone
กงล้อ	k o ŋ ˧ . l ɔː ˦˥

suppose tone map: ˧ -> 2,  ˦˥ -> 3
text: กงล้อ -> กง   ล้อ
phones: [ '_', 'k', 'o', 'ŋ',    'l', 'ɔː', '_']
tones: [ 1, 2, 2, 2,      3, 3, 1]  # 2 copied three times (3 phones), 3 copied twice (2 phones)

Here, I'll ignore the "." for now because I can't figure out what it represents.

"." Is just a delimiter for each syllable.

@BankNatchapol
Thank you for your explanation.
So is the example I gave correct?

Note that the phoneme column in the Wiktionary file includes some graphemes with two phonetic transcriptions separated by a comma. This can happen when a word has multiple accepted pronunciations or when the pronunciation can change based on context or regional accents.
For example, ไอดอล (ไอ=i, ดอล=dol) means "idol," where its phonemes include ʔaj˧.dɔl˥˩, ʔaj˧.dɔn˥˩ --> ʔaj˧.dɔl˥˩ (i-dol) or ʔaj˧.dɔn˥˩ (i-don).

Thank you all for the insightful feedback.
I have pushed some changes and added another test case:
def test_g2p():
    # Test case for the word "กงล้อ"
    text = "กงล้อ"
    normalized_text = text_normalize(text)
    phones, tones, word2ph = g2p(normalized_text)

    # Expected output based on the wiktionary entry
    expected_phones = ['_', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
    expected_tones = [0, 2, 2, 2, 3, 3, 0]
    expected_word2ph = [1, 5, 1]

The TLDR is I now separate the syllables based on ".", then extract the tones and assign values based on this map:
tone_map = {
    "˧": 2,  # Mid tone
    "˨˩": 1,  # Low tone
    "˦˥": 3,  # Rising tone
    "˩˩˦": 4,  # Falling tone
    "˥˩": 5,  # High tone
}

For the tones list, it is calculated similarly to the method @jeremy110 described above, i.e for the word "กงล้อ" results in the following phonemes:  ['k', 'o', 'ŋ', '˧', '.', 'l', 'ɔː', '˦˥'].
...as we can see the first 3 phonemes = 'k', 'o', 'ŋ' have an associated tone of '˧' which corresponds to 2,
and the remains 2 phonemes = 'l', 'ɔː' has a tone of '˦˥' which corresponds to 3,
therefore we get a list that looks like this [0, 2, 2, 2, 3, 3, 0].
Note: that the zeroes at the beginning and end represents the "_" special character.
Another note: If no tone marker is found in a group, a default tone value of 2 (mid tone) is assigned to all the phonemes in that group, i.e these two have the same tone:
กง	k o ŋ
กง	k o ŋ ˧

As for the word2ph list, it represents the number of phonemes for each word in the input text, including special tokens.
So using our previous example,  we get [1, 5, 1] where the ones are special characters and the 5 represents 'k', 'o', 'ŋ', 'l', 'ɔː'.
Note that the word2ph list has a length equal to the number of words plus 2 (for the special tokens).

I should note however that there are surely edge cases and I am not entirely sure if the get_bert_feature function is correct. As always, any feedback and support is very much appreciated 🙏
Happy weekend everyone.

Happy weekend ~~@jadechip
There are still some things that need to be modified
# Expected output based on the wiktionary entry
    expected_phones = ['_', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
    expected_tones = [0, 2, 2, 2, 3, 3, 0]
    expected_word2ph = [1, 3, 2, 1] # modified --> 3 mean 3 phones(koŋ), 2 mean 2 phones(l ɔ)

I have a Thai TTS dataset that is open-source dataset. https://huggingface.co/datasets/lunarlist/edited_common_voice

I have a Thai TTS dataset that is open-source dataset. https://huggingface.co/datasets/lunarlist/edited_common_voice

Any recommendation for fixing G2P? The pythainlp.transliterate.transliterate("สามารถ", engine="thaig2p") usually gives repeated phonemes.

I have a Thai TTS dataset that is open-source dataset. https://huggingface.co/datasets/lunarlist/edited_common_voice

Any recommendation for fixing G2P? The pythainlp.transliterate.transliterate("สามารถ", engine="thaig2p") usually gives repeated phonemes.

Can you try https://huggingface.co/pythainlp/thaig2p-v2.0?

The issue has become a super long discussion! 😃
@jadechip

I should note however that there are surely edge cases and I am not entirely sure if the get_bert_feature function is correct. As always, any feedback and support is very much appreciated 🙏 Happy weekend everyone.

I did a quick check and test on your commits (https://github.com/jadechip/MeloTTS/commit/ffd8f4174cd1ff5d4cbe2ad8bc5e923de8455e75#diff-8f6f83dc5d5f83888cfad03f6835561fd38ec675fd0b5a07b3911ed38d786487).
I hope there was no mistake from my environment.
I found that it could not pass the assertion assert inputs["input_ids"].shape[-1] == len(word2ph), f"{inputs['input_ids'].shape[-1]}/{len(word2ph)}"
I followed the chinese_mix.py, chinese_bert.py. It seems they use character-based tokens for Chinese while using word-based tokens (IMK) for English, so there is no problem when extracting phonemes because it aligns well with the tokenizer from hfl/chinese-roberta-wwm-ext-large
For Thai, we need to be careful about the size of inputs and word2ph since each Thai BERT-based tokenizer can yield different tokens.
For example,
text = "กงล้อ"

 Expected output based on the wiktionary entry
    expected_phones = ['_', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
    expected_tones = [0, 2, 2, 2, 3, 3, 0]
    expected_word2ph = [1, 5, 1]

The tokenizer should tokenize "กงล้อ"  into 1 token (+2 special tokens).
On the other hand,
# Expected output based on the wiktionary entry
    expected_phones = ['_', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
    expected_tones = [0, 2, 2, 2, 3, 3, 0]
    expected_word2ph = [1, 3, 2, 1] # modified --> 3 mean 3 phones(koŋ), 2 mean 2 phones(l ɔ)

The tokenizer should tokenize "กงล้อ"  into 2 tokens, กง and ล้อ  (+2 special tokens)
Ultimately, I think if we handle it properly (e.g., using the tokenizer before G2P), the get_bert_feature  would be fine.
Plus, I am trying to make a rule to combine an initial, vowel, and final phones properly like they did in  chinese_mix.py. The result will look like:
text = "Today ฉันกินข้าว Good เป็นไง"
expected_phones = ['_', 't', 'ah', 'd', 'ey', 'ch', 'an', 'k', 'in', 'kh', 'aaw', 'g', 'uh', 'd', 'p', 'en', 'N', 'aj', _]
expected_tones = [0, 7, 8, 7, 9, 5, 5, 1, 1, 3, 3, 7, 9, 7, 1, 1, 1, 1, 0]
expected_word2ph = [1, 4, 2, 2, 2, 3, 2, 2, 1]

Just as an example, please omit the English tones here.
I still need to deal with the alignment of word2ph and tokens tokenized from BERT. If my method works, I will update.

Thank you all for your valuable feedback! It's great to see such active collaboration, showcasing the strength of the open source community 💪
Apologies @tchayintr, the assertion was indeed failing. I've pushed some code which should resolve the issue, and the output format should now be correct:
BERT tokens: ['▁', 'กง', 'ล้อ']
Aligning word: กงล้อ
Word phonemes: ['k', 'o', 'ŋ', 'l', 'ɔː']
Word tones: [2, 2, 2, 3, 3]
Final phs: ['_', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
Final tones: [1, 2, 2, 2, 3, 3, 1]
Final word2ph: [1, 3, 2, 1]

Thank you for highlighting your proposed approach, please feel free to run some tests or add changes to my code, I am still not 100% confident that everything is working perfectly, but I am eager to try another training job soon to see if the quality has improved 😄
@BankNatchapol do you mean we should replace the g2p function with another model?
I am not sure how much better https://huggingface.co/pythainlp/thaig2p-v2.0 would be as it is trained on the same dictionary  file that I am using (wiktionary-23-7-2022-clean.tsv). Or just let me know if my understanding is incorrect.

Fine tune Thai Example
https://www.youtube.com/watch?v=7sApLg5l2Ps

Test results of fine-tuning the Thai text-to-speech model https://www.youtube.com/watch?v=7sApLg5l2Ps

Thank you for the example, @tiebay004
May I know more details about the libraries or models you used? Coqui?
Honestly, sorry to mention it, but the result doesn't seem as smooth as other languages published on MeloTTS.
I wonder if it is related to MeloTTS? 😭
We are looking forward to the day when the smoothness of Thai TTS is comparable to major languages. However, I feel MeloTTS might have a chance.

Since I had never heard of MeloTTS before, I chose to fine-tune with Coqui. I will try fine-tuning with MeloTTS, but the training takes a long time, and I am using my son's PC for training because he has a GPU with high VRAM.
Since it's the school holidays in Thailand, I should be able to experiment more. As I haven't worked in data before, I might not be able to answer many technical questions about the model. I used to be a developer several years ago.

Hello everyone. I've been a bit busy lately but I've pushed some changes that I think should fix a lot of the outstanding issues. However I am running into some CUDA issues trying to get the training code to run, which wasn't happening before.
terminate called after throwing an instance of 'c10::Error'
  what():  CUDA error: device-side assert triggered
Compile with `TORCH_USE_CUDA_DSA` to enable device-side assertions.

I think this means the code is trying to access elements of a tensor that are outside its valid range but I am not sure where this is happening exactly. Using pdf I was able to pinpoint it down to the forward method of the SynthesizerTrn class in models.py, but I am still not quite sure where the problem lies.
For added context, this is how my train.list file looks like right now.
/workspace/commonvoice/common_voice_th_25686299.wav|TH-default|TH|อนาคตของการทำงานคือมนุษย์หุ่นยนต์ที่เพิ่มขึ้น|_ ʔ a n aː kʰ o t̚ kʰ ɔː ŋ ก า ร ท ำ ง า น kʰ ɯː m a n u t̚ h u n j o n tʰ iː เ พ ิ ่ ม ข ึ ้ น _|1 1 1 2 2 3 3 3 4 4 4 2 2 2 2 2 2 2 2 2 2 3 3 3 3 3 1 1 1 2 2 2 5 5 2 2 2 2 2 2 2 2 2 1|1 10 8 2 5 6 2 9 1
/workspace/commonvoice/common_voice_th_26600610.wav|TH-default|TH|ผู้ประกอบอาชีพ|_ pʰ uː p r a k ɔː p̚ ʔ aː t͡ɕʰ iː p̚ _|1 5 5 1 1 1 1 1 1 2 2 5 5 5 1|1 8 5 1
/workspace/commonvoice/common_voice_th_25686917.wav|TH-default|TH|ฉันหวังว่าทักษะของคุณจะเกาขึ้น|_ t͡ɕʰ a n w a ŋ ว ่ า tʰ a k̚ s aʔ kʰ ɔː ŋ kʰ u n c aʔ k eː า kʰ ɯ n _|1 4 4 4 4 4 4 2 2 2 3 3 3 1 1 4 4 4 2 2 2 2 2 2 2 2 5 5 5 1|1 8 1 5 6 2 2 1 3 1
/workspace/commonvoice/common_voice_th_26665439.wav|TH-default|TH|อันหนึ่งอันเดียวกัน|_ ʔ a n n ɯ ŋ ʔ a n d ia̯w k a n _|1 2 2 2 1 1 1 2 2 2 2 2 2 2 2 1|1 6 3 5 1

@jeremy110 should I update the number of tones in symbols.py to 5 as Thai has 5 tones?
num_th_tones = 1
The reason I am asking is because I see most other languages have the tone set to 1, so I am not sure if it has any impact.

Hello @jadechip
Yes, you should update your number of tones to 5.
There are some issue in your train.list:

The _ at the beginning and end should correspond to tone 0.
The tones and word2ph do not match up with our previous discussion.
To illustrate with my example: three 8's, three 1's, three 2's, two 7's, one 5 ...

/path/audio.wav|F1|TAI|一杯走味的咖啡,|_ ʦ i t p u e ʦ a u b i e k a p i , _|0 8 8 8 1 1 1 2 2 2 7 7 5 1 1 1 1 0 0|1 3 3 3 2 1 2 2 1 1

Thank you @jeremy110, can I just clarify
I have switched the tones for the "_" characters to zeroes.
However I am a bit confused regarding the format of the word2ph list, I should note that I have switched the tokenizer in the g2p method, so the word mapping might be a bit different. Right now it uses the same tokenizer as the get_bert_feature function, which I believe adheres more to the implementation of other languages.
As an example, the phrase ใครเป็นผู้รับ would be tokenized into the following chunks tokenized ['▁ใคร', 'เป็นผู้รับ'].
This results in a word2ph list that looks like this: [1, 3, 8, 1] where the ones are the underscore characters and the 3 = ใ ค ร, and the 8 = เ ป็ น ผ้ ู ร ั บ.
...and there is 13 tones (one assigned for each phoneme).
tokenized ['▁ใคร', 'เป็นผู้รับ']
Final phs: ['_', 'kʰ', 'r', 'aj', 'p', 'e', 'n', 'pʰ', 'uː', 'r', 'a', 'p̚', '_']
Final tones: [0, 2, 2, 2, 2, 2, 2, 5, 5, 3, 3, 3, 0]
Final word2ph: [1, 3, 8, 1]
len(phones) 13
len(tones) 13

Is this correct?

Hello @jadechip
I think you can refer to the French section. Below is an example in French, so you can see that word2ph calculates by converting words into their IPA format phones. In it, sə- corresponds to 3, sɛʁvˈis corresponds to 7, ɡʁatyˈi corresponds to 7, and ɛt corresponds to 2.
French: Ce service gratuit est disponible en chinois simplifié et autres 123.
ipa: sə- sɛʁvˈis ɡʁatyˈi ɛt disponˈibl ɑ̃n ʃinwˈa sɛ̃plifjˈe e otʁz sˈɑ̃ vˈɛ̃ tʁwˈa.

phones: ['_', 's', 'ə', '-', 's', 'ɛ', 'ʁ', 'v', 'ˈ', 'i', 's', 'ɡ', 'ʁ', 'a', 't', 'y', 'ˈ', 'i', 'ɛ', 't', 'd', 'i', 's', 'p', 'o', 'n', 'ˈ', 'i', 'b', 'l', 'ɑ', '̃', 'n', 'ʃ', 'i', 'n', 'w', 'ˈ', 'a', 's', 'ɛ', '̃', 'p', 'l', 'i', 'f', 'j', 'ˈ', 'e', 'e', 'o', 't', 'ʁ', 'z', 's', 'ˈ', 'ɑ', '̃', 'v', 'ˈ', 'ɛ', '̃', 't', 'ʁ', 'w', 'ˈ', 'a', '.', '_']
tones: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
word2ph: [1, 3, 7, 7, 2, 10, 3, 6, 5, 5, 1, 4, 13, 1, 1]

In your case should be
Final phs: ['_', 'kʰ', 'r', 'aj',       'p', 'e', 'n', 'pʰ', 'uː', 'r', 'a', 'p̚', '_']
Final tones: [0, 2, 2, 2,       2, 2, 2, 5, 5, 3, 3, 3, 0]
Final word2ph: [1, 3,      3, 2, 3, 1]

Hello there, how were your training results? Are you still struggling with the pronunciation?

Hi @maryne-ii, I believe the pronunciation issues should be resolved, however I am having some issues getting distributed training to work, this is the error I am getting:
terminate called after throwing an instance of 'gloo::EnforceNotMet'
  what():  [enforce fail at ../third_party/gloo/gloo/transport/tcp/pair.cc:446] op.preamble.length <= op.nbytes. 874668 vs 80644

My understanding is the gloo library is used by PyTorch for collective communication in distributed training, so perhaps the error indicates some kind of mismatch between expected and actual sizes during TCP communication, but I am not sure what in my code - if anything - is causing this...
I should also note I have been using PyTorch > 2.x to train as I was getting other CUDA errors similar to #96.
Training on a single GPU seems to work though.

I've had some time to continue working on this and was able to resolve the training issues.
I believe the inconsistencies were caused by the tokenizer I was using. I have now changed to a tokenizer that more aligns with the format expected by the codebase. The format is close to what @jeremy110 suggested apart from the underscore characters in the output. I am not sure if I should remove the underscore characters in the tokenized text before calculating the phs, tones and word2ph values? I am concerned it might cause inconsistencies with the get_bert_feature function later in the pipeline?
The tokenized text ['▁', 'กง', 'ล้อ']
Final phs: ['_', '▁', 'k', 'o', 'ŋ', 'l', 'ɔː', '_']
Final tones: [0, 2, 2, 2, 2, 3, 3, 0]
Final word2ph: [1, 1, 3, 2, 1]
bert features shape torch.Size([768, 8])

I think it can be removed because it duplicates the original underscores.

Ok, thank you, I will give this a shot.

hello @jadechip
I'm doing something similar to you. I want to train Khmer and Lao, but the dialogue 120 is too long, and I keep getting errors in train.py. I'm planning to write a training tutorial for a small language, can you help me? I'd appreciate it

Thanks for your continued interest and support.
I found quite a few issues in the g2p lookup logic which should now be resolved, and I have changed the tokenizer to clicknext/phayathaibert which I believe should be more aligned with the tokenizers of other languages.
  I tried several times remove them but they caused mismatches in the get_bert_feature function, so I decided to keep it for now.
Unfortunately the training loss is still far from ideal and the model is not producing great results. I was able to get it down to around 55 by tweaking the learning rate but I am not sure what is prohibiting it from going lower.

I have pushed all my changes so if anyone would like to give training a go, please let me know how it goes 🤞

Here are the results from the different loss components of my lates training run, side by side with the results from @jeremy110 posted. The ones on the left in red are mine. Based on my limited understanding, the g/fm loss is increasing, which seems to suggest the generator is struggling to match the discriminator's features.

I will try another run with a different learning rate decay and more aggressive gradient clipping. I might also need to normalize the audio.

@Zengyi-Qin Sounds good, will report back once I have proper training results.

Hey can you send me your info so that I can reach out to you regarding the implementation of this project for a new language.I am facing alot of issues


Trying to get simple test working, ReferenceError: Request is not defined

I am following your example test here:
https://github.com/honojs/hono-minimal/blob/main/src/index.test.ts
I have pretty much the same simple test here:
https://github.com/lane711/yoga/blob/feature/hono2/src/index.test.ts
but when I run it I get:
` FAIL  src/index.test.ts
● Test the application › Should return 200 response
ReferenceError: Request is not defined

   6 | describe('Test the application', () => {
   7 |   it('Should return 200 response', async () => {
>  8 |     const res = await app.request(`${baseUrl}/test`)
     |                           ^
   9 |     expect(res.status).toBe(200)
  10 |   })
  11 | })

  at Hono.request (node_modules/hono/dist/cjs/hono.js:66:28)
  at Object.<anonymous> (src/index.test.ts:8:27)`

nvm I was able to get this working by starting with the honor minimal project


Custom author name not reflecting in summary

Summary:
Custom author name preference should reflect the text value change in the summary, instead of repeating the information in the summary of Use custom author name switch preference.
Steps to reproduce:
How can we reproduce the issue?
Go to Settings and enable the Use custom author name. Change the name by tapping on the Custom author name preference.
What did you expect the app to do, and what did you see instead?
I expected the current custom name to be reflected in the summary of that preference. However, the summary is fixed redundant information. I say redundant because we have already conveyed the meaning of the preference to the user.
Device and Android version:
Redmi Note 5 Pro
MIUI version 11
Android 9
Commons app version:
master and betaDebug
Screen-shots:
Changing Custom author name preference

After tapping OK

Would you like to work on the issue?
Yes. If we can agree that the issue is genuine, then I'd love to start working on it.

Yes, I like the idea of displaying the author's name instead of repeating the text. Also, let us merge both these settings ie "Use custom author name" and "Custom author name".
The behavior can be:

when "use custom author name" is switched enabled, let us show the dialog box where the user can edit the author name.
when the switch is disabled, don't use the custom author name.

@maskaravivek Good idea. Can I start working on this?

Also, let us merge both these settings ie "Use custom author name" and "Custom author name".

That's a good thing to do but ...

The behavior can be:
* when "use custom author name" is switched enabled, let us show the dialog box where the user can edit the author name.

* when the switch is disabled, don't use the custom author name.

... IMO, merging this the other way round would be more intuitive. By other way round I mean merging "Use custom author name" into "Custom author name". That way, we would only have the "Custom author name" option.

When a user has not set a custom author name, we show the description of the option in the summary.
When a user has set a custom author name, we show the author name that has been set in the summary.
Tapping on the option would open the dialog to enter a custom author name.

... IMO, merging this the other way round would be more intuitive. By other way round I mean merging "Use custom author name" into "Custom author name". That way, we would only have the "Custom author name" option.

The approach that I suggested would also have just 1 option ie "use custom author name". Maybe we can improve the copy text. With this approach, the user can simply toggle off the setting to stop using the custom author name instead of opening the dialog box, erasing the custom name and saving it. When the toggle is off, we can continue to store the custom author name is the shared prefs and the user can come back later and enable it again. He could choose to either edit it or continue using the previously set author name.

When a user has not set a custom author name, we show the description of the option in the summary.
When a user has set a custom author name, we show the author name that has been set in the summary.
Tapping on the option would open the dialog to enter a custom author name.

The issues I see with this approach are:

that to disable the custom author name, the user will have to erase the custom author name and save it.
our logic that checks and uses custom author names would change. It will have to start checking for empty values. On the other hand with the approach that I had proposed, it would just be a UI change and our key/value pairs in the shared prefs would continue to have the same meaning and behavior.

The approach that I suggested would also have just 1 option ie "use custom author name".

Yeah, I understood that initially.

Maybe we can improve the copy text. With this approach, the user can simply toggle off the setting to stop using the custom author name instead of opening the dialog box, erasing the custom name and saving it. When the toggle is off, we can continue to store the custom author name is the shared prefs and the user can come back later and enable it again. He could choose to either edit it or continue using the previously set author name.

That sounds useful. The reason I found merging the options into a toggle option unintuitive is that it would make the toggle option do more than just toggling an option. That's why I was suggesting to merge the other way round. Alternatively, keeping both the options and just update the summary sounds good too.

When a user has not set a custom author name, we show the description of the option in the summary.
When a user has set a custom author name, we show the author name that has been set in the summary.
Tapping on the option would open the dialog to enter a custom author name.

The issues I see with this approach are:
* that to disable the custom author name, the user will have to erase the custom author name and save it.

Yeah. We could provide an option to easily clear the author name via an 'x' shown near the text field.

* our logic that checks and uses custom author names would change. It will have to start checking for empty values. On the other hand with the approach that I had proposed, it would just be a UI change and our key/value pairs in the shared prefs would continue to have the same meaning and behavior.

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

It would matter for people who have already installed the app, have the custom author name set but the toggle disabled. Once the app updated, their custom author name will start getting used which might not be what they wanted.

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

It would matter for people who have already installed the app, have the custom author name set but the toggle disabled. Once the app updated, their custom author name will start getting used which might not be what they wanted.

Ah! That's an issue indeed. We can think of a migration plan to handle this. During the migration,

If the "Use custom author name" switch is turned-off, we clear the author name filled by the user (if any)
If the "Use custom author name" switch is turned-on, we just use the author name provided by the user

Just a suggestion 🙂

True, but UX wise I would prefer having a toggle instead of making the user clear the custom author name.
Let us get some more opinions and then a volunteer can pick the task.

True, but UX wise I would prefer having a toggle instead of making the user clear the custom author name.

Yeah, disabling a toggle would indeed be better. I suppose we should just keep the two option as it is then. That would be a lot better, IMO.

Let us get some more opinions and then a volunteer can pick the task.

Sure.

Personally I think, a toggle would be simpler in terms of UX, disabling the author name by a switch surely seems better than making the user manually clear it.

Yeah, disabling a toggle would indeed be better. I suppose we should just keep the two option as it is then. That would be a lot better, IMO.

I agree with @sivaraam, the toggle will perhaps become unintuitive if it would do more than just toggling. I have rarely seen the toggle doing more. For example, in the official Reddit app, there's an option to toggle the dark mode and a separate option to choose the dark mode theme. This is similar to our issue. I can imagine it getting slightly annoying if the app asked me my dark theme preference every time I turned on dark mode.

@maskaravivek How should we proceed with this?

Fixed in #3536


Possibility to tp randomly to a player after some time

Is your feature request related to a problem? Please describe.
No, I just thought this may be handy for some players.
Describe the solution you'd like
The possibility to let the bot tp to a random person if it has permissions after some time that has been set.
Describe alternatives you've considered
N/A
N/A

In case if you administrator of target server you can just use command block for this purpose for instance.


Basic implementation of scripted_modifier

Work done:

[X] Basic implementation of scripted_modifier type
[X] Basic implementation of ai_target_modifier subtype
[X] Added subtype to ai_chance_block

Related to #21 . Probably would be a good idea for someone to checkout both of these PRs, merge them, and PR the resulting merge.
Will do this later tonight or tomorrow if I have time, but may not get to it so if someone wants to beat me to it, go right ahead.

Closed this one because I accidentally pushed under a different user.


Add option to customize theme

Since I already use variables everywhere, it should be as simple as changing them from local to the global scope.

@oranja @edouard-lopez what's the best way to achieve this?

For bare fish (not using any framework), I believe the most standard way is to rearrange the variable assignments to serve as defaults in case the variable is not yet set.
For example:
# Symbols
set -q symbol_prompt; or set -g symbol_prompt "❯"
set -q symbol_git_down_arrow; or set -g symbol_git_down_arrow "⇣"
set -q symbol_git_up_arrow; or set -g symbol_git_up_arrow "⇡"
set -q symbol_git_dirty; or set -g symbol_git_dirty "*"
set -q symbol_horizontal_bar; or set -g symbol_horizontal_bar "—"

This way, any of these variables can be defined in config.fish without being overwritten by pure's fish_prompt.fish.
If it gets too tedious, you can define a helper function:
function set_default -S -a var value
    set -q $var; or set -g $var $value
end

Then the previous block becomes:
# Symbols
set_default symbol_prompt "❯"
set_default symbol_git_down_arrow "⇣"
set_default symbol_git_up_arrow "⇡"
set_default symbol_git_dirty "*"
set_default symbol_horizontal_bar "—"

In the price of indirection you get a more compact block, and it also helps to avoid bugs from typos...

@oranja that looks pretty decent and straightforward. Will give it a try! Thanks man.

Agree with @oranja, but please always prefix any global variables with the name of your plugin/theme/prompt/snippet/etc so as to not conflict (hardly, but still) with other global variables of the same name.

Great, I will keep that in mind. Thanks.
--
Rafael Rinaldi
http://rinaldi.io

On Feb 13, 2016, at 15:27, Jorge Bucaran<EMAIL_ADDRESS>wrote:
Agree with @oranja, but please always prefix any global variables with the name of your plugin/theme/prompt/snippet/etc so as to not conflict (hardly, but still) with other global variables of the same name.
—
Reply to this email directly or view it on GitHub.

Thanks for your knowledge!
@oranja good proposal
@bucaran nice improvement! Love future-proof argument!
don't we need -g to set globally?

What @bucaran is suggesting is to have two variables for each customization options.
The first is local and is the one actually used in the theme code.
The second can have any scope that affects the prompt - universal, global, or local to an outer function (i.e. a test function).
To clarify, here's @bucaran's suggestion again, with annotations. (hoping I won't get sued for copyright infringement here).
set -l symbol_prompt ❯    # This is the local variable that will be used in the prompt's code.

if set -q pure_symbol_prompt    # Here we check for any previous definition of `pure_symbol_prompt`. Not necessarily global.
    set symbol_prompt $pure_symbol_prompt    # We override the local variable with the external value, if it's present.
end

One more thing that I don't know whether it supports my argument or not, is that fish does this too, so it's a way to stay consistent across the universe.
https://github.com/fish-shell/fish-shell/blob/master/share/functions/__fish_config_interactive.fish#L18

don't we need -g to set globally?

Depends. I was assuming this variable will only be used in the file where it was declared.

In the same file fish also has several of these set -q var; or set var
one liners, rendering your choice argument weaker.
Having said that, both are fine, but personally, I think that if you have
many of these in a row, it's better to use the shorter form. It's an
equally acceptable fish idiom.
On Tue, Feb 16, 2016, 00:22 Jorge Bucaran<EMAIL_ADDRESS>wrote:

One more thing that I don't know whether it supports my argument or not,
is that fish does this too, so it's a way to stay consistent across the
universe.
https://github.com/fish-shell/fish-shell/blob/master/share/functions/__fish_config_interactive.fish#L18
don't we need -g to set globally?
Depends. I was assuming this variable will only be used in the file where
it was declared.
—
Reply to this email directly or view it on GitHub
https://github.com/rafaelrinaldi/pure/issues/2#issuecomment-184420949.

I think this issue should be closed by #30.

@Shougo That's true. Thanks!


Add more information for docker setup

Checklist

[X] I agree to the terms within the Code of Conduct.

Describe the problem you'd like to have solved
I needed to use this in my docker-compose configuration, but the documentation's docker setup section was not very helpful.
Describe the ideal solution
The ideal solution is documentation which uses the exporter in a docker environment.
Alternatives and current workarounds
I solved this issue already. Will open a PR.
Additional context
No response

Thank you for raising the ticket and making the PR! I didn't consider the docker-compose use case.


fix(plugins): ai proxy cloud identity transformers

Summary

Fixed a bug where cloud identity authentication was not used in ai-request-transformer and ai-response-transformer plugins.
Re-factor "cloud SDK auth" function to be shared across many AI plugins.

Checklist

[x] The Pull Request has tests - already has them
[x] A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
[ ] There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - already are docs

Issue reference
AG-94

@fffonion @liverpool8056 I done all the comments. I think many is just that I haven't commented it so well.

@tysoekong It looks like this PR includes parts of https://github.com/Kong/kong-ee/pull/9930/files plus other changes. Could you rework commits in such a way that it closely matches https://github.com/Kong/kong-ee/pull/9930/files (including changelog) and any addition is in its own separate commit? As it's right now, we would have to squash and merge, which would result in unrelated commits being squashed.

@gszr Again this is deliberate! It is because there are EE features in this plugin, that are only adjusted in https://github.com/Kong/kong-ee/pull/9930/files
It's all in the plan.

@gszr Again this one is deliberate! It is because there are EE-only features in this plugin, that are only adjusted in Kong/kong-ee#9930 (files)
It's all in the plan.
I will rebase ee 9930 when this gets picked.

@tysoekong please squash last commit onto the one above it so we can rebase and merge.


Reference search + some app hover states

before:

After:

some hover effects
mobile fixes for client url


email validation broken with uppercase letters

If the first letter of the domain name is capitalized, it won't validate.

Is that a valid email?

yep, a user ran into this and was confused.

@sean I recommend dropping https://github.com/lbryio/lbry-desktop/blob/master/ui/constants/email.js
Validation already happens server-side, so there is little value in validating client side aside from UX reasons.
If you want to do something simple to improve UX client-side, I'd reduce the regex to something like: /^[^@]+@[^@]+$/ (there's something non-@ before an @, there's an @, and there's something non_@ after the @).

Changed to the simpler email regex


CL-EL withdrawals harmonization: using units of Gwei

Following discussion from here https://github.com/ethereum/pm/issues/702, protocol devs want to explore harmonization in the withdrawals format from CL to EL.
This PR updates the Engine API with the first part of this change to use the same units (Gwei) for the amount in each withdrawal.

See EIP update here: https://github.com/ethereum/EIPs/pull/6325

lgtm


KAFKA-3080: Fix ConsoleConsumerTest by checking version when service is started

The MessageFormatter being used was only introduced as of <IP_ADDRESS>. The Kafka
version in some tests is changed dynamically, sometimes from trunk back to an
earlier version, so this option must be set based on the version used when the
service is started, not when it is created.

@granders Tracked this down to 9220df9f8b58870a2282d3e4ceb2e003667d854b. Verified this fixes the failing test and still passes all of security_rolling_upgrade_test. Maybe @benstopford can sanity check this as well since he was the author on the original patch?
And perhaps @gwenshap for final review + commit?

Thanks for fixing this @ewencp, makes sense to me.

Looks good, passes locally.
Not expecting anything new, but branch builder is running here:
http://jenkins.confluent.io/job/kafka_system_tests_branch_builder/338/

@granders there were some failures in the branch builder. Are they related?

@ijuma
There were 2 failures, but as far as I can tell they seem unrelated:
This failed, but also failed on trunk on 1/17/2016, so I believe it is unrelated
Module: kafkatest.tests.zookeeper_security_upgrade_test
Class:  ZooKeeperSecurityUpgradeTest
Method: test_zk_security_upgrade
Arguments:
{
  "security_protocol": "SASL_PLAINTEXT"
}

This also failed
Module: kafkatest.tests.benchmark_test
Class:  Benchmark
Method: test_consumer_throughput
Arguments:
{
  "security_protocol": "SSL"
}

Failure was due to a parsing error (parsing statistics choked on a NotLeaderForPartitionException), but the same test with "interbroker_security_protocol": "PLAINTEXT", "security_protocol": "SSL" passed. If the failure was due to Ewen's change, I would have expected it to fail on "interbroker_security_protocol": "PLAINTEXT" as well.

Running again here:
https://jenkins.confluent.io/job/kafka_system_tests_branch_builder/341/

@ewencp @gwenshap All tests passed:
http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2016-01-18--001.1453165281--ewencp--kafka-3080-system-test-console-consumer-version-failure--0f0d33f/report.html

Thanks @granders. It would be great to get this in @gwenshap as it fixes a bunch of system test failures.

LGTM. It would be great to get this in.

LGTM


tf.fromPixels() is renamed to tf.browser.fromPixels(),You can disable deprecation warnings with tf.disableDeprecationWarnings().

Hi All , facing the following issue , while locating the element with ai.
[debug] [WD Proxy] Got response with status 200: "{"sessionId":"5b49b524-d285-4e48-a12b-e74bd1e1bea3","status":0,"value":"iVBORw0KGgoAAAANSUhEUgAAAtAAAAXwCAYAAAB7XHwkAAAABHNCSVQICAgIfAhkiAAAIABJREFU\neJzs3XdYVGfePvCb3mFARES62FAZFQtqIrZYUmxE3USDUdM22RA1ebPvu\/vGRLO7v3eLJWbT14ik\nGjWiaWqighU7oIIFpKoUlaF35vfHOMcZ5kw5MDS5P9eVKzPnnJnzMDPCPc95nu9jYe0\/WQkiIiIi\nIjKJZUc3gIiIiIioK2GAJiIiIiKSgAGaiIiIiEgCBmgiIiIiIgkYoImIiIiIJGCAJiIiIiKSgAGa\niIiIiEgCBmgiIiIiIgkYoImIiIiIJGCAJiIiIiKSgAGaiIiIiEgCBmgiIiIiIgkYoImIiIiIJGCA\nJiIiIiKSgAGaiIiIiEgCBmgiIiIiIgkYoImIiIiIJGCAJiIiIiKSgAGaiIiIiEgCBmgiIiIiIgkY\noImIiIiIJGCAJiIiIiKSgAGaiIiIiEgCBmgiIiIiIgkYoKnTc3FyhLOTg8nHy1yd4eLk2OL9RERE\nnc1L0bPx3394Gg72dh3dFAJg3dENoI7l7uaC0vIKODs6wMrKCqXlFWhqUmod80zUNMx7dALmLv9f\nYduuzX\/B9z8fxhc797dZ2\/oF++KFp59AcEBvKJXAtax8fPrVD8jKvSV6\/LiRQ\/BM1CPo1dMDAJB3\nswifff0jLl7OMmk\/ERFRZ2FvZwtraytUVFbD3c0FkRFy2NvZwsc7EZnZN8x2nvCwAZg1bRz6B\/vB\n3s7WbM9rTE1tHa5ez8Oe\/cdxNvWKyY9zdLCDlaUVyiur2rB1xjFAd2N9vD3h4uwIRVkFfH284Ozk\ngOSL11DX1NDRTQMAxCydB6+e7vj+58OwsrLE44+MwytL5uCNdz\...
[ai] Turning screenshot into HTML image for use with canvas
[ai] Getting screenshot slices for each element
[ai] Making label predictions based on element images
tf.loadFrozenModel() is going away. Use tf.loadGraphModel() instead, and note the positional argument changes. You can disable deprecation warnings with tf.disableDeprecationWarnings().
============================
Hi there 👋. Looks like you are running TensorFlow.js in Node.js. To speed things up dramatically, install our node backend, which binds to TensorFlow C++, by running npm i @tensorflow/tfjs-node, or npm i @tensorflow/tfjs-node-gpu if you have CUDA. Then call require('@tensorflow/tfjs-node'); (-gpu suffix for CUDA) at the start of your program. Visit https://github.com/tensorflow/tfjs-node for more details.
tf.fromPixels() is renamed to tf.browser.fromPixels(), please switch to the new method as the old method will be removed in TensorFlow.js 1.0. You can disable deprecation warnings with tf.disableDeprecationWarnings().
[ai] Predictions for 29 element(s) took 345.222 seconds
[ai] Found 0 matching elements
[ai] Resetting element response attribute setting to original value: ""
[UiAutomator2] Forwarding the following settings to the UiAutomator2 server: ["elementResponseAttributes"]
[debug] [WD Proxy] Matched '/appium/settings' to command name 'updateSettings'

@BSudheerR i don't see any issues here in the log, other than the warning from tensorflow, which isn't fatal. but it does suggest maybe something wasn't installed correctly, since @tensorflow/tjfs-node is indeed a dependency of this project, and is required here

Thanks @jlipps . npm i @tensorflow/tfjs-node solves my problems. Also This one "[ai] Found 0 matching elements" is because the cart element which i am trying is unable to find my plug in , even after changing the matching confidence to 0.1 , Not sure if it is issue with model or cart Image matching .. HOwever I could able to match with other elements and perform Action.

OK, good to know. Thanks!


InitialRating for multiple select elements with same id or class

Hey guys...
Looking for a way to add initialRating via data-current-rating having a list of multiple select elements....
$(".rating").barrating({
    theme: 'fontawesome-stars-o',
    initialRating: $(this).attr('data-current-rating') // does not work
});

This does not work ...

This definitely won't work as you expect as this may be something else, based on from where you initialise plugin.
Safe would be to loop through all .rating and apply barrating.
$('.rating').each(function(index, el) {
  var $El = $(el);
  $El.barrating({
    theme: 'fontawesome-stars-o',
    initialRating: $El.attr('data-current-rating')
  });
});

@ChetanGoti How do you apply this in the HTML or EJS files please? What do you need to pass in the function?


Feature: Add Repo During Install of Deb Package for Automatic Updates

There's either an obvious reason for this not being the case or it was an oversight but it would be handy if on Debian based Linux OSes we'd get updates and upgrades every time we ran apt-get update && apt-get upgrade.

Thanks for taking the time to contribute!
We noticed that this is a duplicate of https://github.com/atom/atom/issues/2956. You may want to subscribe there for updates but also see this comment for the most general comment about the issue status: https://github.com/atom/atom/issues/2956#issuecomment-264260509.
Because we treat our issues list as the Atom team's backlog, we close duplicates to focus our work and not have to touch the same chunk of code for the same reason multiple times. This is also why we may mark something as duplicate that isn't an exact duplicate but is closely related.
For information on how to use GitHub's search feature to find out if something is a duplicate before filing, see the How Can I Contribute? section of the Atom CONTRIBUTING guide.


Automator: update<EMAIL_ADDRESS>in<EMAIL_ADDRESS>

Generated by Automator - 2023-06-26T18:31:01+00:00

@istio-testing: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

release-notes_istio_release-1.18
f3d44d888bdacf7eda7666cb1f0ce70d6e0430e7
link
true
/test release-notes

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Release notes jobs are stuck for release branches until the new logic is installed. That is stuck right now behind some issues with common-files jobs not running.

/test release-notes


My game causes an error upon exiting when it does not have an audio device connected

Tested versions
4.3 dev 5
System information
Godot v4.3.dev5.mono - Windows 10.0.22621 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 (NVIDIA; <IP_ADDRESS>76) - Intel(R) Core(TM) i7-9700KF CPU @ 3.60GHz (8 Threads)
Issue description
My game causes an error upon exiting when it does not have an audio device connected.
Turning on my speaker works normally if there is an audio device connected.

Steps to reproduce

start
exit
error

Minimal reproduction project (MRP)
N/A

I guess we can remove the ERR_FAIL_COND_V(hr != S_OK, ERR_CANT_OPEN); in AudioDriverWASAPI::audio_device_init, it's checked at outside anyway.


Integrate Brocade.io as a fallback for OFF

URI: ferrisoxide/brocade.io
Integrate Brocade.io as a Fallback for OFF

[ ] Test integration

[ ] Find matching if-cause to use the alternate API
[ ] Test on test server

[ ] Git Push
[ ] Productive Integration

Example item (all fields used): Nutella


example .editorconfig and documented Visual Studio default should match

In nearly all example .editorconfig in this document the configured value matches the Visual Studio default, but not everywhere.  It would be helpful if it would be possible to take the examples from this page, and have a .editorconfig that is functionally identical to the Visual Studio default, to make it easier to align all editors that support editorconfig to the Visual Studio default.
Case in point here:
the example states
csharp_indent_labels = flush_left
but the Visual studio default states:
no_change

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: db54afa5-fc7b-394f-ede3-4d1d46ec695d
Version Independent ID: 86441bf6-9e5b-85c2-d42d-3afd5a9a069b
Content: .NET formatting conventions for EditorConfig - Visual Studio
Content Source: docs/ide/editorconfig-formatting-conventions.md
Product: visual-studio-windows
Technology: vs-ide-general
GitHub Login: @jillre
Microsoft Alias: jillfra

VS defaults are different between profiles and which settings you have roaming. I do not think we can provide a 'default' editorconfig

@jmarolf In that case I wonder what the "Visual Studio Default" item is doing in the documentation? My point is, either make sure the example editorconfig sections are filled with the value stated as Visual Studio default value, or remove the Visual studio default value.
On the other hand, it is not that big a deal. Closing this issue as won't fix would make sense to me as well.

@mikadumont FYI on this.

@rjvdboon yeah, I agree. I don't think its helpful @TerryGLee can you remove the Visual Studio default sections from the table?

@jmarolf There's a PR ready for you to review. :)

@rjvdboon The PR with the fix should be live tomorrow by noon. I'm closing this issue now; thanks for helping us improve the documentation!


Return claims from ValidateRequest

Claims are really useful, why not return them from ValidateRequest along with the token in JWTValidator?
ValidateRequest(r *http.Request) (*jwt.JSONWebToken, *jwt.Claims, error)

I understand that's an API change, but it's a good one.

The claims are the content of the JWT so the ValidateRequest only needs to validate if the request it's correct and return the corresponding jwt or error.
When you have the jwt with a nil error it's the moment you can extract the claims with:
func (v *JWTValidator) Claims(r *http.Request, token *jwt.JSONWebToken, values ...interface{}) error
I think the design of the functionality it's correct in order to separate the responsibilities in different functions.


Python Virtual Environment folder is created

Python Virtual Environment folder is created. This folder is not needed. Hence must be untracked.

This is resolved in PR #20


err handling TODO done

the err handling code was changed from fmt to panic

Thanks for your contribution

Thanks for your contribution


remove superfluous A=zeros(s);

@mparsani reported a problem with the changes of #40. Here, I try to fix this by removing a superfluous line of code.
I did some local tests before opening #40 on my workstation and my notebook with two different versions of MATLAB and didn't notice any problems. However, I hope that this fixes the problems with other versions of MATLAB.

Superseded by 83f70620152e699cf56d63eb89b6ad38ba67658b.


Issuer-signed JWT Verification Key Validation - Separation of signature and identity verification/validation?

There seem to be several mechanisms for issuer key validation (section 3.5).
Two mechanisms define fetching of keys (issuer metadata, DID), and one can be embedded or referenced (x509).
Would it make sense to enable signature validation at all times and

require a jwk header with the issuer's signing key (https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.3)
Signature is always verified using this key.

Public key - identity binding can be verified

using x509 certificate that is in the jwk.x5c claim (https://datatracker.ietf.org/doc/html/rfc7517#section-4.7) - key certificate or the full chain.

kid could be misused to express the validation mechanism or one could define an additional header claim where identity verification mechanism is specified:

calling the /.well-known/jwt-vc-issuer
resolving a DID
using Federation ...
...

I think this makes sense only in the case of embedded X509 certificate.
Otherwise, I think the "validation mechanism" (e.g. resolving the DID or downloading the keys from a web server) would always have to be executed anyway. There would be no extra value in having an embedded signing key, since an attacker that can replace the signature can also replace the signing key. And there would be a risk that implementers create security holes by considering the "validation mechanism" as something optional...
Or am I missing anything?

Excellent point @peacekeeper!
There are always 2 processes

Verifying the signature: By embedding a key, the signature can be always verified. This does not exclude an option to resolve the key.
Verifying the signer's identity: each method defines it's own process: x509 embedded or referenced (referenced x509 is almost never used); .well-known -> resolve the URL, DID -> resolve the DID (URL)

In many environments (high-loads, low connectivity, ...) we cannot rely on ". resolving the DID or downloading the keys from a web server would always have to be executed anyway". Furthermore, if the resource is not available, again, signature validation fails. Hence, these mechanisms (well-known, DIDs) should also define how identity information could be embedded in the signature (like x509, OIDFed, advanced signatures are already doing) - but this is out of scope of this specification and discussion.

Thanks @alenhorvat

By embedding a key, the signature can be always verified.

Yes, but what's the value of knowing that a payload has been signed by "someone", if you don't know who that key belongs to?

define how identity information could be embedded in the signature

Yes that sounds useful!

require a jwk header with the issuer's signing key (https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.3)
Signature is always verified using this key.

We discussed this topic with the other editor's. Our opinion is that including the jwk in the header would be quite dangerous since it would encourage implementations to always use the jwk and skip all the other validation checks.
@alenhorvat  Is there a specific use case you have in mind that would be blocked by this?

Related question:
What prevents a malicious issuer to define an issuer identifier: happy-issuer.com/issuers/google.com?
How can the Verifier/RP know whether issuer is truly Google or not?

Challenges

RP needs to resolve a key to verify a signature - issues for offline cases
if RP needs to verify a high number of VCs issued by different issuers, results with additional effort to resolve the keys
managing and resolving historical keys/key rotation -- you can have several key rotation in the lifecycle of a credential.

Implementers MUST understand how to verify the identity, independently of the signature validation. This also forces the standard to clearly define the two verification streams.

I think one way to address this is as follows and you have eluted to the same thing in your previous post:

use an X.509 chain with a trusted root cert
include the end-entity certificate in the x5c header that signs the SD-JWT
end-entity certificate is issued by trusted root cert

This way, a verifier can cache the trusted root cert and use the contained end-entity cert in the x5c to validate the X.509 chain and verify the signature.
For simplicity I didn't mention intermediate certs but they can be included in the x5c header as well.
This behaviour is enforced by X.509 itself (RFC5280) which requires certificate chain validation. If implementations don't have a trust point, the x5c would not be trusted and therefore rejected.
It seems you want to define the same thing for jwk?  Why is using X.509 not an option in this case?
If you want to use DIDs and/or issuer metadata, wouldn't it be possible to support the use cases you just mentioned by simply caching DID resolution or the issuer metadata URL?

IMO x5c/x5u+x5t are enough, but this document puts x509, jwk+URL, and DID on the same level.
It might work for some DID methods and it might work if JWK "trust model" is clearly defined; e.g., see the example above where a malicious actor lists an issuer as one of their clients. It can be translated to an x509 model, where RP must define which "Root JWKs" it trusts (e.g., service providers that offer VC issuance as a service and URL contains 2 identifiers, of the provider (e.g., mattr) and the actual issuer (e.g., university of wonderland))

Related question: What prevents a malicious issuer to define an issuer identifier: happy-issuer.com/issuers/google.com?
How can the Verifier/RP know whether issuer is truly Google or not?

Nothing, but I believe this issue is solved by the trust framework and the provided trust points, isn't it? There are various mechanisms and based on my prior observations of different trust frameworks, there can be a lot of differences.
In your specific example which I think has to be corrected (since an attacker would need to use Google's domain in the issuer identifier and not happy-issuer.com, to pretend to be Google), I believe the trust point is Internet PKI and if you don't trust the Internet PKI, it probably makes no sense to rely on issuer well-known alone or at all.
You can combine this with X.509 (or even DIDs if required).

In your specific example which I think has to be corrected (since an attacker would need to use Google's domain in the issuer identifier and not happy-issuer.com, to pretend to be Google)

wouldn't the "iss" value be: happy-issuer.com/issuers/google.com? I don't need to know google's domain to do that. I only need my domain.

Perhaps we are talking past each other but I thought you were concerned about impersonating Google in your example. I was saying that there is no reason for a verifier to think that the identity of happy-issuer.com/issuers/google.com is Google if it doesn't use one of Google's domains, right?

What is google's identifier if they are using service hosted by the happy issuer?


add support for follow copy limit

done in https://github.com/GetStream/stream-php/commit/41ca74098f02e137a1406192a1ce458754764088#diff-f68460d5bd58c9da13499bc2b81ac408


_reduxReactFirebase.firebase.storage is not a function(…)

How can I access the storage using the redux-react-firebase object? With the original firebase NPM works fine.
I'm trying like this: let imgRef = firebase.storage().child('posts/' + post.slug + '.jpg');
Thanks

currently you can't, but I will try to add this or you can make a PR

@tiberiuc my pr #40 is solving that.

@RahavLussato I've installed the latest version 2.3.5 (which includes your PR), but I still have the same error. How exactly does your PR fix this issue?

its because the child function is not on storage is on storageref you need to change your code to something like:
let storageRef =  firebase.storage().ref();

let imgRef = storageRef.child(type + '/' + item.slug + '.jpg');
    imgRef.getDownloadURL().then(function(url) {
        this.refs[className+'-'+id+'-img'].src = url;
    }.bind(this)).catch(function(error) {
        console.log(error);
});

https://firebase.google.com/docs/storage/web/create-reference

@RahavLussato thanks! it worked :)

@tiberiuc you can close this

@RahavLussato somehow, this stopped working: this.storageRef = this.props.firebase.storage().ref();
I'm getting now Uncaught (in promise) TypeError: Cannot read property 'storage' of undefined

@gazpachu try to get firebase from props this.props.firebase and don't forget to use the firebase decorator.


Design Meeting Notes, 1/25/2023

ignoreDeprecations in TS 4.9
https://github.com/microsoft/TypeScript/issues/51909

importsNotUsedAsValues and preserveValueImports are deprecated in favor of verbatimModuleSyntax
You're supposed to use the ignoreDeprecations flag to temporarily disable this as you migrate off the flags.
Can't make a tsconfig.json that works for both 4.9 and 5.0.
Nice to be able to let a team move over incrementally.
Want to bring ignoreDeprecations back to 4.9

Helps with editor scenarios too while VS Code still ships 4.9.

Moving from barrel imports in our codebase
https://github.com/microsoft/TypeScript/pull/51455
https://github.com/microsoft/TypeScript/pull/51590

In our migration to modules, we use barrel imports to keep things simple-ish, keep initialization order.
Big task, but @a-tarasyuk took it on.
Had to resolve issues with cycles, seems like it's fine.

Gives some benefits that now some tools can analyze our codebase to detect cycles.

Makes moving code around harder.

But that is an empathy gap between us an our users.
We should think about editor features to help here in 5.1
Also, would be nice if we could detect cycles in some way.

Some instances where we still use (ts as any).Yadda for preserved const enums.
By explicitly declaring what the parser imports from, other tools can more-easily tree shake from us.
Also #51440

Previously, these attempts ran into crashes due to circularities.
Fixing the cycles would allow many of these to operate correctly.

So #51455

Seems uncontroversial.

#51440

Something to be said about flat files.
Would be nice to ship an ESM version

Moving to direct imports?

Seems okay.

Import Sorting
https://github.com/microsoft/TypeScript/pull/52090

ESLint doesn't tie-break sorting well - i.e. if you import hasDecorators and HasDecorators, the two might flip.

So it's not a stable sort!?
No.

We made more internationally-sensitive changes in #51733.

Compatible with what? ESLint doesn't give deterministic (or is it stable?) sorting, we do.
Based on eslint/sort-imports.

They used toUppercase, we did toLowercase for case insensitivity.
Noticeable for ASCII characters in between - namely the underscore (_)

#52115

Uses Intl.Collator to do sorting if desired.
Compatible with eslint-plugin-simple-import-sort.

Does Prettier do import sorting?

No.
Thankfully. Don't need 30 tools fighting with each other. 😅

dprint does if you want, and it's compatible!
Sounds like we have good-enough coverage to say "we support all of them."
So okay with changing our repo to new linter options?

Seems fine.

Aside: need to copy settings over for auto-imports(?)

Helps with editor scenarios too while VS Code still ships 4.9.

Shouldn't they use local workspace TS 5.0 version? Or is it about web only case?

IIRC, VS Code doesn't use the workspace version until you opt into it - but either way, your team might be using a local workspace version of 4.9 while you are trying to upgrade to 5.0.

IIRC, VS Code doesn't use the workspace version until you opt into it

Yeah, by default VS Code uses whatever TypeScript version it ships with (currently 4.9). This is a security feature, because the locally installed TypeScript version could potentially be malicious code.

Prettier can sort imports via a plug-in - we use it because it has better overall support than eslint. https://github.com/trivago/prettier-plugin-sort-imports

Hopefully the natural-sort function is compatible with the organizeImportsNumericCollation from https://github.com/microsoft/TypeScript/pull/52115.


Debug health check

Motivation and context

How has this been tested?

Checklist

[ ] I submit my changes into the develop branch
[ ] I have added a description of my changes into CHANGELOG file
[ ] I have updated the documentation accordingly
[ ] I have added tests to cover my changes
[ ] I have linked related issues (read github docs)
[ ] I have increased versions of npm packages if it is necessary (cvat-canvas,
cvat-core, cvat-data and cvat-ui)

License

[ ] I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.

@azhavoro , can we customize the error message in the PR?

@azhavoro , could you please resolve the conflict?


rewrite cvtsrcpf to support dbcs

Fixes #106

[ ] doc update

What is the root cause of #106 and how does this fix it?

What is the root cause of #106 and how does this fix it?

The original method uses qsh and cat to read a physical and redirects the output to the streamfile. For files encoded with DBCS, the cat gets blank files. In this pr, I use CPYTOSTMF.

What is the root cause of #106 and how does this fix it?

The original method uses qsh and cat to read a physical and redirects the output to the streamfile. For files encoded with DBCS, the cat gets blank files. In this pr, I use CPYTOSTMF.

Makes sense. Thank you

@TongkunZhang
I have met with Barbara Morris and Liam Allan to discuss how to handle the CCSID problem.
The situation is:
ILE RPG and ILE C support TRTCCSID and COBOL and CL do not yet.
Tooling will require all source to be encoded in UTF8 to avoid NLS problems during editing.
However the source needs to be in the right EBCDIC.
So when we convert to IFS we will store  the CCSID of the source SRC-PF in the target directory .ibmi.json
If the SRC-PF is 65535, then the value of the -defaultCcsid parameter of the cvtsrcpf command will be used (used what spelling/case that makes sense_
If the parameter is not specified the job CCSID should be used.
During compilation, even CL and ILE COBOL will use the CRTFRMSTMF trick using a SRC-PF created in QTEMP.  The SRC-PF will be created with the CCSID specified in .ibmi.json
Only ILE RPG and ILE C can be safely compiled from stream files at this point.
The only issue remaining is long source lines in CL.
Use + continuation as per https://www.ibm.com/docs/en/i/7.4?topic=parts-cl-command-continuation to split lines to the record length of the created SRC-PF.
Make the SRC-PF a long record length like 512 and this should not even have to happen very often.

@TongkunZhang I have met with Barbara Morris and Liam Allan to discuss how to handle the CCSID problem. The situation is: ILE RPG and ILE C support TRTCCSID and COBOL and CL do not yet. Tooling will require all source to be encoded in UTF8 to avoid NLS problems during editing. However the source needs to be in the right EBCDIC. So when we convert to IFS we will store the CCSID of the source SRC-PF in the target directory .ibmi.json If the SRC-PF is 65535, then the value of the -defaultCcsid parameter of the cvtsrcpf command will be used (used what spelling/case that makes sense_ If the parameter is not specified the job CCSID should be used.
During compilation, even CL and ILE COBOL will use the CRTFRMSTMF trick using a SRC-PF created in QTEMP. The SRC-PF will be created with the CCSID specified in .ibmi.json Only ILE RPG and ILE C can be safely compiled from stream files at this point.
The only issue remaining is long source lines in CL. Use + continuation as per https://www.ibm.com/docs/en/i/7.4?topic=parts-cl-command-continuation to split lines to the record length of the created SRC-PF. Make the SRC-PF a long record length like 512 and this should not even have to happen very often.

At least at 7.5 the TGTCCSID keyword for CRTRPGMOD and CRTCMOD seem to be addressing different things.  (Or maybe the help is not correct)
C TGTCCSID Specifies the target coded character set identifier used to describe data stored into the resulting module object.
RPG TGTCCSID Specifies the CCSID that the compiler uses to read the source files.

At least at 7.5 the TGTCCSID keyword for CRTRPGMOD and CRTCMOD seem to be addressing different things. (Or maybe the help is not correct) C TGTCCSID Specifies the target coded character set identifier used to describe data stored into the resulting module object. RPG TGTCCSID Specifies the CCSID that the compiler uses to read the source files.

So it looks like we need to use the pragma convert to accomplish this for C
https://www.wisdomjobs.com/e-university/ibm-ile-tutorial-183/converting-string-literals-in-a-source-file-5674.html
Is this something that we should insert as part of converting to stream files?

slack conversation with Barbara Morris
The doc says that #pragma convert only applies to literals https://www.ibm.com/docs/en/i/7.5?topic=descriptions-convert#prag_convert
https://www.ibm.com/docs/en/i/7.5?topic=tc-how-ile-cc-compiler-converts-source-file-target-ccsid
When the TGTCCSID differs from the source file's CCSID, the ILE C compiler converts the source files to the TGTCCSID and processes files. This ensures that the target module and all it's character data components (for example, listing, string pool) are in the desired TGTCCSID. You can then develop in one character set and target another. The argument defaults to the source file's character set so the default behavior is backward compatible (with the exception of 290, 930 and 5026).
BUT
Note: C++ converts only the string literals (not the source) to the TGTCCSID.
I don't know what that means for import/export function names
https://www.ibm.com/docs/en/i/7.5?topic=ccsid-literals-comments-identifiers
The TGTCCSID parameter allows you to choose the CCSID of the resulting module. The module's CCSID identifies the coded character set identifier in which the module's character data is stored, including character data used to describe literals, comments and identifier names described by the source (with the exception of identifier names for CCSIDs 5026, 930 and 290).
So for ILE C, it seems like the actual implementation of TGTCCSID is the same as for ILE RPG.
The question for the C++ compiler folks is how identifier names are handled for UTF-8 source.
And I guess find out what are the exact caveats for 5026, 930, and 290.

@TongkunZhang In conclusion we will be using TGTCCSID parameter for C/C++ and ILE RPG and CRTFRMSTMF for all the other languages

https://www.ibm.com/support/pages/node/1108071 using TGTCCSID(*JOB) might be appropriate if 65535 is the source SRC-PF CCSID

@TongkunZhang  can we merge in this change and close the issue?


Response Model Descriptions are incorrectly set and overwritten.

When an endpoint is generated in the swagger doc the description field of the response model is set incorrectly:
@definitions[response_model][:description] = description_object(route)

The description_object(route) method returns the description of the route, which can be really long and is different for each route!
So if you have an endpoint, say, getKittens:
desc 'Get all kittens!', {
  nickname: 'getKittens',
  success: Entities::Kitten
  failure: [[401, 'KittenBitesError', Entities::BadKitten]]
  }
get '/kittens' do
   puts "meow"
end

Grape Swagger will define the new models Kitten and also BadKitten and will apply the same description in the the definition. Swagger generated doc:
definitions:
  kitten:
    type: object
    properties:
      fluffy:
        type: boolean
        description: Indicates if the kitten is fluffy or not!
    required:
      - fluffy
    description:  Get all kittens!

If you use the KittenEntity as a response somewhere else, then the model definition description will get overwritten once more with the description of the route.
Since the description of the route is in no way useful for the model description I suggest we simply replace it with a generic description:
@definitions[response_model][:description] = "A #{response_model} model"

@toshe … yeap it makes sense to me, a PR would be nice 😉

closed via #804 https://github.com/ruby-grape/grape-swagger/pull/804


Creating formulae that work for both homebrew and linuxbrew

Currently, I'm doing it in the following way:
class Antibody < Formula
  desc "A faster and simpler antigen written in Golang."
  homepage "http://getantibody.github.io"
  url "https://github.com/getantibody/antibody/releases/download/v2.2.2/antibody_#{%x(uname -s).gsub(/\n/, '')}_#{%x(uname -m).gsub(/\n/, '')}.tar.gz"
  head "https://github.com/getantibody/antibody.git"
  version "v2.2.2"

 # etc
end

I find this very hacky and ugly. Is there any better way of doing this?

In cases where the URL is different for Linux and Mac, I usually prefer to do this:
if OS.mac?
  url "..."
elsif OS.linux?
  url "..."
end

That also allows you to have different SHA-256's for each download and works for all the different things people might call macOS releases (I've variously seen "darwin", "osx", "mac", etc. For Linux it's pretty reliably just "linux").
Still, I agree that it's not great. If it's just one link to update every time you need to bump the formula, it's no big deal; if it's several, it can quickly become a pain.
@sjackman, any better suggestions?

@rwhogg that will solve the uname -s part, but not the uname -m (x86_64, i386, etc)...
But, yeah, looks better already...


I see nothing in the demo page

I was trying to test the demo from your official tutorial website. I logged in with my facebook account. But no pictures are loaded. How is that ?

Do you see any errors in the console?
Do you have any photos in your account that it can load?
Did you give the application permission to access your photos?

Same issue. Here is the console output:
playcanvas-stable.min.js:17 Powered by PlayCanvas 0.186.9 939b059
playcanvas-stable.min.js:17 WARNING: Invalid address mode in U set on non power of two texture. Forcing clamp to edge addressing.
playcanvas-stable.min.js:17 WARNING: Invalid address mode in V set on non power of two texture. Forcing clamp to edge addressing.
playcanvas-stable.min.js:17 WARNING: Invalid minification filter mode set on non power of two texture. Forcing linear addressing.
playcanvas-stable.min.js:17 WARNING: Invalid address mode in U set on non power of two texture. Forcing clamp to edge addressing.
playcanvas-stable.min.js:17 WARNING: Invalid address mode in V set on non power of two texture. Forcing clamp to edge addressing.
``

Do you have any photos in your account that it can load?
Did you give the application permission to access your photos?

To answer your questions: yes and yes.
I just went back through the process of removing the PlayCanvas Login Sample app from my facebook app list and re-permitted the app when playing the demo to no avail. I experience the same console logging as @kkanzelmeyer.
When I pull up the App within the App Settings menu on facebook it says that the app can use my name, profile picture, age, and 'other public info'.
Other than permitting the app is there a specific profile Security Setting that must be set properly to get this demo to work?

This application is using the Graph API to read a list of your photos. It uses this endpoint /me/photos to get the list of photos. Then iterates that list and gets the URLs of each photo.
You could try using the Facebook Graph Explorer to see if /me/photos/ will return anything for you? https://developers.facebook.com/tools/explorer/

It returns a ton of entries like the one below:
"data": [     {       "created_time": "2016-01-19T00:46:08+0000",       "id": "533605200151347"     },

Can you try running this app: https://playcanv.as/b/rsHWdvo0/
It prints "received X photos" then "found photo" for each URL it gets in the console.

Output:
received 0 photos

Any progress on this issue?

My suspicion is that this is an issue with the permissions you have given the application or have setup on your FB account. The application uses the official Facebook Javascript SDK to retrieve the list of photos and you are receiving 0 photos. I'm not sure this is an application bug.
You could try and use the FB JS SDK to access /your-user-id/photos and /photo-id?fields=images to see if you get a different result using your own app?


Errors sent in response to restartFrame are not shown to user

This was previously raised in #87605, but the fix doesn't seem to be working for me.
I'm setting supportsRestartFrame during initializeRequest:
response.body.supportsRestartFrame = true;

And I'm just always returning an error in restartFrameRequest:
protected restartFrameRequest(response: DebugProtocol.RestartFrameResponse, args: DebugProtocol.RestartFrameArguments): void {
	response.success = false;
	response.message = `Failed to restart frame !!`;
	this.sendResponse(response);
}

However no message appears anywhere when trying to restart a frame:

@isidorn

@DanTup thanks for filling this. Now we show an error notification


Running benchmarks once per year is not sufficient

Search for duplicate feature request

[X] I already searched, and this feature request or improvement is not a duplicate.

Feature scope
Build target (release binary)
Feature request related to a problem
As I’m implementing changes right now, I don’t really like guessing about performance impact. I’d like to see it if my changes cause a performance regression.
Describe the solution you'd like
I want to try running benchmarks via a Github Action, so that we have some data for each change. Yes, the action runner isn’t a platform with reliable hardware characteristics. However, if we also measure lighttpd/nginx/apache for comparison, this might be good enough to detect at least significant performance degradations.
Describe alternatives you've considered
It would of course be possible to get a cloud server with known hardware parameters for this task, it should produce more reliable results. But that’s probably an overkill for now.
Build target
All targets
Additional context
No response

Yes, there is also people wanting to see updates about this like https://github.com/static-web-server/benchmarks/issues/3.
Regardless, I agree that in order to improve performance and prevent regressions, we need some continuous benchmarking solution.
For example, we could set up / refactor our CI workflows and run our microbenchmarks continuously. Another alternative is a  cloud solutions like https://bencher.dev/ that are free for public repositories or any other.
But the CI itself has to be improved for example to run the microbenchmarks, use valgrind to check for memory leaks (e.g.  https://github.com/tokio-rs/tokio/blob/master/.github/workflows/ci.yml#L273), etc.
And now that the code is getting more visibility on it's own place (modules). We could better optimize and incorporate benchmarks via criterion progressively.
I do not expect to benchmark every single peace of code of course but the hot parts or where makes sense.
BTW, there here are a few for future reference.
In the other hand, the benchmark repo is an HTTP load testing benchmark comparison that could complement the whole picture but can be also automated.
Definitely, there is room for improvement in the performance front. Feel free to propose something in a draft.

@palant now that https://github.com/static-web-server/static-web-server/pull/355 can we close this?

@joseluisq @palant Any chance of adding Caddy to the benchmarks list?

@joseluisq @palant Any chance of adding Caddy to the benchmarks list?

Sure, I’ve created #379.
Caddy performance results are surprising. It ranks really well when compressing large files – but it doesn’t compress well. Even when I set compression level to 9 I get 22 KiB instead of the expected 18 KiB. In all other benchmarks it ranks consistently worse than SWS, and it consistently produces nasty outliers where latency is much higher than usual. Directory listings in particular are extremely slow (also quite a bit more complex than what other servers show).

Ah, forgot to restart after config change apparently, makes sense now. With compression level 9 I do get the expected 18 KiB. But the default compression level seems to be 1, same as nginx. While I could reconfigure them, using a lower compression level in SWS is actually a better idea.