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

Refactor MLLM #529

Open
wants to merge 132 commits into
base: main
Choose a base branch
from
Open

Refactor MLLM #529

wants to merge 132 commits into from

Conversation

hhaAndroid
Copy link
Collaborator

In order to enhance the flexibility and usability of the multimodal model, it is necessary to refactor the existing llava code.

val_dataset = [
dict(
type=MMELLaVADataset,
data_file='/mnt/petrelfs/huanghaian/code/xtuner/LMUData/MME.tsv',
Copy link
Collaborator

Choose a reason for hiding this comment

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

像这种路径,是让用户修改config来设置,还是说像VLMEvalKit一样,内部自动下载

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我觉得不用这么智能吧,直接要他们提前下载好,省的出现一些奇怪问题

# set log processor
log_processor = dict(by_epoch=False)

# ==================== val and test cfg =======================
Copy link
Collaborator

Choose a reason for hiding this comment

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

此部分,是放到上面 PART 3 Dataset & Dataloader 还是单独放在下面?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我觉得单独放下面,用户用起来更方便?

print_log('============================================', 'current')
print_log(score, 'current')
print_log('============================================', 'current')
print_log(f'YOrN_eval successfully finished evaluating', 'current')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
print_log(f'YOrN_eval successfully finished evaluating', 'current')
print_log('Hallusion successfully finished evaluating', 'current')

with pd.ExcelWriter(osp.join(work_dir, self.results_xlsx_path), engine='openpyxl') as writer:
results_df.to_excel(writer, index=False)

score = Hallusion_rating(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hallusion_rating 函数放到本文件? 因为这个应该只有这里会用到,所以方便查看。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

可以

with pd.ExcelWriter(osp.join(work_dir, self.results_xlsx_path), engine='openpyxl') as writer:
results_df.to_excel(writer, index=False)

score = MME_rating(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

MME_rating 放到本文件下?

Comment on lines 29 to 31
template = prompt_template
self.template = template

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
template = prompt_template
self.template = template
self.template = prompt_template

Comment on lines 32 to 34
template = prompt_template
self.template = template

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
template = prompt_template
self.template = template
self.template = prompt_template

Comment on lines 33 to 35
template = prompt_template
self.template = template

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
template = prompt_template
self.template = template
self.template = prompt_template

Comment on lines 65 to 67
template = prompt_template
self.template = template

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
template = prompt_template
self.template = template
self.template = prompt_template

Comment on lines 45 to 47
template = prompt_template
self.template = template

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
template = prompt_template
self.template = template
self.template = prompt_template

results = collect_results(results, len(dataset))
self.runner.logger.info('========= Starting the evaluation of a data ===========')
if is_main_process():
metric = dataset.evaluate(results, self.runner.work_dir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

self.runner.work_dir 是否考虑携带 iter 信息,类似这里

def _save_eval_output(self, runner, eval_outputs):
save_path = os.path.join(runner.log_dir, 'vis_data',
f'eval_outputs_iter_{runner.iter}.txt')
with open(save_path, 'w', encoding='utf-8') as f:
for i, output in enumerate(eval_outputs):
f.write(f'Eval output {i + 1}:\n{output}\n\n')

print_log('Samples: {}, Accuracy: {:.2f}%'.format(len(pred_list), acc), 'current')
print_log('============================================', 'current')
print_log(f'TextVQA successfully finished evaluating', 'current')
return {'acc': acc}
Copy link
Collaborator

Choose a reason for hiding this comment

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

return acc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

规定必须要返回字典吧,我记得,因为到时候 bset checkpoint 时候会读取

@@ -96,7 +97,11 @@ def main():
runner = RUNNERS.build(cfg)

state_dict = guess_load_checkpoint(args.checkpoint)
Copy link
Collaborator

Choose a reason for hiding this comment

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

checkpoint 现在是一个 optional argument,看起来是之前残留的bug,最好改成 positional argument。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@Mountchicken
Copy link

Hi @hhaAndroid
Thanks for your great work! May I ask if this branch can work now.

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

Successfully merging this pull request may close these issues.

4 participants