ADD: Raise warning for duplicated conversation types

[JohanWork]

Rasise a warning if conversation is specified but already exists in the type for datasets. This is for example the case for ultra_apply_chatml.

[winglian]

Aren't the chatml.* already builtin formatters? I would think we wouldn't want to warn for those since they are correct.

[winglian]

Suggested change

	"chatml.intel",
	"chatml.argilla",

Aren't these current formatters? I don't think we should expect the user to receive a warning if they use these two.

[JohanWork]

chatml.intel is used here https://github.com/OpenAccess-AI-Collective/axolotl/blob/5787e1a23f61be093fc26067f8a05342fbdfbe1a/tests/e2e/test_dpo.py#L46 and the test seams to work. So thought they where supported but might miss something :) chatml.argilla is mention here https://github.com/OpenAccess-AI-Collective/axolotl/blob/5787e1a23f61be093fc26067f8a05342fbdfbe1a/docs/rlhf.md?plain=1#L29

[winglian]

Right, if the chatml.* types.work, then we shouldn't we warning here? Or am I misunderstanding something?

[JohanWork]

@winglian I don't fully understand. My intention was to suggest that we should issue a warning or raise a ValueError if a conversation format is added, but there's already a type in place that formats the data. This situation arises, for example, with ultra_apply_chatml (see the example in the comment below). Here's another example where the conversation format is determined by the type: https://huggingface.co/cognitivecomputations/dolphin-2.6-mistral-7b-dpo/blob/5c32e515f3d79beefc110e8a07c3671269a0f5ab/configs/dolphin-dpo.yml#L14

[winglian]

I don't think we ever had toxic_apply_chatml in our previous dpo release pre-cleanup.

[JohanWork]

At least from how I've understood the DPO (and this took me some time, and I might have misunderstood it):

datasets:
    path: argilla/ultrafeedback-binarized-preferences-cleaned
    split: train
    type: ultra_apply_chatml
    conversation: chatml 

This doesn't make any sense. You shouldn't set conversation here since the type actually also includes formatting for a ChatML conversation. In some cases, type serves both as the input format and the format used for conversation style, to my understanding. Therefore, I thought it would be valid to raise an error if you use a type format that also includes a conversation format and your setting conversation.

Regarding toxic_apply_chatml do you mean it is not supported or was previously not supported?

[JohanWork]

@winglian, I now understand your point. However, I still believe that setting conversations for DPO should not be possible since it's done in type. Or am I still overlooking something?