Skip to content

Conversation

@xnuohz
Copy link
Contributor

@xnuohz xnuohz commented Oct 30, 2025

Issue

Close #10514, #10529

CodeCov

Before

image

After

image

@xnuohz xnuohz changed the title Improve .llm code coverage [Code Coverage] llm/models/sentence_transformer.py and llm/models/vision_transformer.py Oct 31, 2025
@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.53%. Comparing base (c211214) to head (b143673).
⚠️ Report is 138 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10516      +/-   ##
==========================================
+ Coverage   86.11%   87.53%   +1.41%     
==========================================
  Files         496      510      +14     
  Lines       33655    35960    +2305     
==========================================
+ Hits        28981    31476    +2495     
+ Misses       4674     4484     -190     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xnuohz xnuohz changed the title [Code Coverage] llm/models/sentence_transformer.py and llm/models/vision_transformer.py Improve .llm code coverage Nov 1, 2025
@xnuohz
Copy link
Contributor Author

xnuohz commented Nov 8, 2025

@puririshi98 @akihironitta .llm test coverage has been successfully uploaded to codecov. ready for review and merge.

Copy link
Contributor

@puririshi98 puririshi98 left a comment

Choose a reason for hiding this comment

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

lgtm, just plz address my one comment

batch_unique = batch.unique()
batch_size = len(question)
if len(batch_unique) < batch_size:
if len(batch_unique) <= batch_size:
Copy link
Contributor

Choose a reason for hiding this comment

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

why less than equal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to test coverage when they are equal, forgot to remove, will update.

Copy link
Contributor

@puririshi98 puririshi98 left a comment

Choose a reason for hiding this comment

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

1 more

model_name='Qwen/Qwen3-0.6B',
num_params=1,
dtype=torch.bfloat16,
sys_prompt='You are an agent, answer my questions.',
Copy link
Contributor

Choose a reason for hiding this comment

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

can you actually run the molGPT example w this change and share a full log of it? just want to make sure everything is still smooth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

root@7df2f109d384:/workspace/pytorch_geometric# python examples/llm/molecule_gpt.py 
Setting up 'Qwen/Qwen3-0.6B' with configuration: {'revision': 'main', 'max_memory': {0: '23GiB'}, 'low_cpu_mem_usage': True, 'device_map': 'auto', 'torch_dtype': torch.bfloat16}
Some weights of RobertaModel were not initialized from the model checkpoint at DeepChem/ChemBERTa-77M-MTR and are newly initialized: ['pooler.dense.bias', 'pooler.dense.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
Total Preparation Time: 9.311407s
Training beginning...
Epoch: 1|3: 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:00<00:00,  4.71it/s]
Epoch: 1|3, Train loss: 2.322746, Val loss: 2.416511
Epoch: 2|3: 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:00<00:00,  8.08it/s]
Epoch: 2|3, Train loss: 1.376676, Val loss: 2.370785
Epoch: 3|3: 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:00<00:00,  7.97it/s]
Epoch: 3|3, Train loss: 1.102427, Val loss: 2.344596
Total Training Time: 4.702314s
Test loss: 0.022353
Total Time: 14.047626s

Copy link
Contributor

Choose a reason for hiding this comment

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

can you run master brnach as well for comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

root@7df2f109d384:/workspace/pytorch_geometric# python examples/llm/molecule_gpt.py 
Setting up 'TinyLlama/TinyLlama-1.1B-Chat-v0.1' with configuration: {'revision': 'main', 'max_memory': {0: '23GiB'}, 'low_cpu_mem_usage': True, 'device_map': 'auto', 'torch_dtype': torch.bfloat16}
Some weights of RobertaModel were not initialized from the model checkpoint at DeepChem/ChemBERTa-77M-MTR and are newly initialized: ['pooler.dense.bias', 'pooler.dense.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
Total Preparation Time: 8.718750s
Training beginning...
Epoch: 1|3:   0%|                                                                                                  | 0/4 [00:00<?, ?it/s]/workspace/pytorch_geometric/torch_geometric/llm/models/molecule_gpt.py:158: UserWarning: HuggingFace model TinyLlama/TinyLlama-1.1B-Chat-v0.1 is not using a chat template, using Llama 2 style prompting. Please consider using a more recent model and initialize the LLM with `sys_prompt`.
  ) = self.llm._get_embeds(instructions, additional_text_context, xs,
Epoch: 1|3: 100%|██████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:01<00:00,  3.85it/s]
Epoch: 1|3, Train loss: 1.763808, Val loss: 2.043718
Epoch: 2|3: 100%|██████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:00<00:00,  6.02it/s]
Epoch: 2|3, Train loss: 1.431526, Val loss: 1.987978
Epoch: 3|3: 100%|██████████████████████████████████████████████████████████████████████████████████████████| 4/4 [00:00<00:00,  6.16it/s]
Epoch: 3|3, Train loss: 1.353049, Val loss: 1.963581
Total Training Time: 6.796006s
Test loss: 0.242709
Total Time: 15.544912s

Copy link
Contributor

Choose a reason for hiding this comment

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

i notice the test los is 10x smaller now while the val and training loss havent changed much between branches. could you rationalize why this is the case? it indicates to me there may be a bug at test time in the new branch unless you can explain why this might be to me

@xnuohz
Copy link
Contributor Author

xnuohz commented Nov 18, 2025

@puririshi98 I forgot to force reload MoleculeDataset when llm switched to Qwen, so the text in the dataset was generated by TinyLlama but trained it with Qwen.
See the training log from scratch below.

master branch from scratch

Setting up 'TinyLlama/TinyLlama-1.1B-Chat-v0.1' with configuration: {'revision': 'main', 'max_memory': {0: '21GiB'}, 'low_cpu_mem_usage': True, 'device_map': 'auto', 'torch_dtype': torch.bfloat16}
Some weights of RobertaModel were not initialized from the model checkpoint at DeepChem/ChemBERTa-77M-MTR and are newly initialized: ['pooler.dense.bias', 'pooler.dense.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
Total Preparation Time: 3359.429658s
Training beginning...
Epoch: 1|3:   0%|                                                                                      | 0/1682 [00:00<?, ?it/s]/workspace/pytorch_geometric/torch_geometric/llm/models/molecule_gpt.py:158: UserWarning: HuggingFace model TinyLlama/TinyLlama-1.1B-Chat-v0.1 is not using a chat template, using Llama 2 style prompting. Please consider using a more recent model and initialize the LLM with `sys_prompt`.
  ) = self.llm._get_embeds(instructions, additional_text_context, xs,
Epoch: 1|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [04:12<00:00,  6.67it/s]
Epoch: 1|3, Train loss: 1.020544, Val loss: 0.994869
Epoch: 2|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [04:11<00:00,  6.69it/s]
Epoch: 2|3, Train loss: 0.816425, Val loss: 0.960044
Epoch: 3|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [04:10<00:00,  6.70it/s]
Epoch: 3|3, Train loss: 0.795707, Val loss: 0.943275
Total Training Time: 778.760076s
Test loss: 0.957731
Total Time: 4144.875072s

this pr from scratch

Setting up 'Qwen/Qwen3-0.6B' with configuration: {'revision': 'main', 'max_memory': {0: '22GiB'}, 'low_cpu_mem_usage': True, 'device_map': 'auto', 'torch_dtype': torch.bfloat16}
Some weights of RobertaModel were not initialized from the model checkpoint at DeepChem/ChemBERTa-77M-MTR and are newly initialized: ['pooler.dense.bias', 'pooler.dense.weight']
You should probably TRAIN this model on a down-stream task to be able to use it for predictions and inference.
Total Preparation Time: 5493.621828s
Training beginning...
Epoch: 1|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [03:21<00:00,  8.37it/s]
Epoch: 1|3, Train loss: 0.581263, Val loss: 0.575342
Epoch: 2|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [03:20<00:00,  8.38it/s]
Epoch: 2|3, Train loss: 0.435040, Val loss: 0.553491
Epoch: 3|3: 100%|███████████████████████████████████████████████████████████████████████████| 1682/1682 [03:20<00:00,  8.38it/s]
Epoch: 3|3, Train loss: 0.405096, Val loss: 0.549858
Total Training Time: 622.549687s
Test loss: 0.585572
Total Time: 6122.272548s

Copy link
Contributor

@puririshi98 puririshi98 left a comment

Choose a reason for hiding this comment

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

okay since the loss has dropped 2x on the test branch i think this is safe to merge just need CI to be green

@xnuohz
Copy link
Contributor Author

xnuohz commented Nov 26, 2025

@puririshi98 ready to merge. the same nightly PyTorch CI error as the master branch.

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.

Improving torch_geometric.llm Code Coverage

2 participants