Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding llama3 fastchat conversation monkeypatch #1539

Merged
merged 4 commits into from
May 10, 2024

Conversation

TJ-Solergibert
Copy link
Contributor

@TJ-Solergibert TJ-Solergibert commented Apr 19, 2024

Description

Adding monkey patch for llama3 chat fine-tuning

Motivation and Context

For instruction fine-tuning llama3 instruct with axolotl awesome features like sample packing, 4d masks, etc.!

How has this been tested?

Llama3 chat template:
{% set loop_messages = messages %}{% for message in loop_messages %}{% set content = '<|start_header_id|>' + message['role'] + '<|end_header_id|>\n\n'+ message['content'] | trim + '<|eot_id|>' %}{% if loop.index0 == 0 %}{% set content = bos_token + content %}{% endif %}{{ content }}{% endfor %}{{ '<|start_header_id|>assistant<|end_header_id|>\n\n' }}

Debugging example:
Screenshot 2024-04-19 at 01 12 30
From this sample:
{"conversations": [{"from": "system", "value": "You are an AI assistant. You will be given a task. You must generate a detailed and long answer."}, {"from": "human", "value": "Write an article based on this \"A man has been charged with murder and attempted murder after a woman and the man she was on a date with were stabbed at a restaurant in Sydney, Australia.\"", "weight": 0.0}, {"from": "gpt", "value": "Title: Tragedy Strikes in Sydney: Victims Stabbed during Dinner Date, Suspect Charged with Murder and Attempted Murder\n\nIntroduction\n\nA seemingly ordinary evening took a tragic turn in Sydney, Australia when a romantic dinner date at a local restaurant ended in bloodshed, as a man allegedly attacked a couple, stabbing both victims. Local authorities have charged the suspect with murder and attempted murder in connection with the horrific crime. In this article, we delve into the chilling incident, the suspect, and the impact it has had on the community.\n\nThe Incident\n\nThe alarming event took place at a popular restaurant in Sydney, Australia. A woman and her date were enjoying their meal \u2013 blissfully unaware of the tragic circumstances that would soon unfold. As they shared conversation and indulged in their dinner, a man approached the couple and, without warning, began to stab them with a sharp object.\n\nPatrons at the restaurant scrambled for safety, as the assailant continued his violent rampage. Bystanders quickly called emergency services, who promptly arrived at the scene. Although both victims were rushed to a nearby hospital, the woman succumbed to her injuries. The man she was on a date with remained in critical condition, fighting for his life.\n\nThe Suspect\n\nInitial investigations revealed that the alleged attacker had no known connections to the victims \u2013 adding to the mystifying nature of this sudden and brutal assault. Witnesses reported that the assailant seemed to have no specific motive and appeared to carry out the act senselessly.\n\nFollowing a thorough investigation, local police identified and arrested the suspect. During the process, it was discovered that the alleged attacker had a history of criminal behavior and a troubled past, though it is unclear if this played a role in the tragic incident.\n\nAuthorities have formally charged the man with murder and attempted murder in connection with the heinous crime. He awaits a hearing to determine a trial date and, if convicted, could face a life sentence in prison.\n\nThe Community's Response\n\nThe shocking nature of the crime has left the residents of Sydney reeling, as they struggle to come to terms with the harrowing event. The restaurant where the attack occurred has since been closed, with a makeshift memorial being created outside to commemorate the victims.\n\nMany have questioned how such a vicious crime could happen in what is considered to be one of the safest cities in the world. This tragic event has spurred local officials to reassess current security measures and devise strategies to reinforce public safety. Additionally, discussions surrounding mental health and criminal rehabilitation have surfaced as residents seek to comprehend the actions of the alleged perpetrator and prevent similar incidents from occurring in the future.\n\nIn the wake of the stabbing, the community has banded together with an outpouring of grief and support for the victims and their families. Candlelight vigils have been held, and an online fundraising campaign is underway to assist the surviving victim with his medical expenses and recovery.\n\nConclusion\n\nThe tragic attack in Sydney serves as a chilling reminder that senseless acts of violence can happen anywhere and at any time. The community's response to this horrific and seemingly random act of brutality has been one of solidarity and determination to prevent such incidents in the future. As the case unfolds, the victims and their families remain in the hearts of the community, who are grieving the devastating loss of a life cut tragically short and supporting the recovering victim as he continues to endure this unimaginable ordeal.", "weight": 1.0}]}

Linked to lm-sys/FastChat#3257

Copy link
Collaborator

@winglian winglian left a comment

Choose a reason for hiding this comment

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

Lgtm. Thanks!

@NanoCode012
Copy link
Collaborator

Is there some commit missing? I don't see registering the llama-3 conversation. I think this requires waiting for upstream to merge first.

@Haoxiang-Wang
Copy link
Contributor

It seems one needs to install the FastChat PR lm-sys/FastChat#3257 by @TJ-Solergibert first before running this PR. @NanoCode012

Let's wait for FastChat to merge @TJ-Solergibert 's PR first.

@TJ-Solergibert
Copy link
Contributor Author

TJ-Solergibert commented Apr 20, 2024

I've push an update to make it compatible with this FastChat PR which is more complete. I first created this PR in the middle of a rush, that's why I didn't pay attention to details. Now I've passed the pre-commit tests.

We used to fine-tune llama-3 in a medical domain, check this for the results.

As @Haoxiang-Wang said, we first need to merge into main of FastChat at least the register_conv_template(Conversation(..)) to make everything work, so I hope it merges soon.

PD: We get exactly the same result with the new FastChat PR (Same text, length)
Screenshot 2024-04-20 at 18 08 05

@TJ-Solergibert
Copy link
Contributor Author

FastChat PR already merged.

@winglian

@xzuyn
Copy link
Contributor

xzuyn commented Apr 25, 2024

The yield "", "<|begin_of_text|>" + system_prompt might result in double bos tokens since the model repo has been updated to correctly add the bos token itself.

@NanoCode012
Copy link
Collaborator

Hey @TJ-Solergibert , looking at this, I think you may need to update the fast chat commit in requirements, check if the BOS needs to be removed per prior comment. Providing a sample debug output would be helpful as well.

@TJ-Solergibert
Copy link
Contributor Author

Hi!

Thanks @NanoCode012 & @xzuyn for your tip, I was already working on other things (Everything moves too fast 😵‍💫). Just pushed the fix, this is the conversation we got before:
Screenshot 2024-05-01 at 17 51 45
And after:
Screenshot 2024-05-01 at 17 56 18

Toni

@TJ-Solergibert
Copy link
Contributor Author

Cc @winglian

@winglian
Copy link
Collaborator

winglian commented May 9, 2024

@TJ-Solergibert Looks like there is a double <|begin_of_text|><|begin_of_text|> in your screenshot above
Screenshot 2024-05-09 at 1 35 26 PM

@TJ-Solergibert
Copy link
Contributor Author

@TJ-Solergibert Looks like there is a double <|begin_of_text|><|begin_of_text|> in your screenshot above

Screenshot 2024-05-09 at 1 35 26 PM

There are 2 images, It's the before and after the patch comparisons!

@winglian winglian merged commit b32c08f into axolotl-ai-cloud:main May 10, 2024
7 checks passed
@Nero10578
Copy link
Contributor

Nero10578 commented May 10, 2024

Are we supposed to see the bos and eos token when using --debug? I don't see it when I run preprocess --debug.

#1608 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants