Skip to content

Conversation

@sam-hey
Copy link
Contributor

@sam-hey sam-hey commented Feb 23, 2025

This PR allows the use of push_to_hub with the PyLate template.

ST lib is importing its own create ModelCard function that is not getting overwritten by PyLate.


from pylate import models
from pylate.models.colbert import PylateModelCardData

model = models.ColBERT(
    model_name_or_path="colbert-ir/colbertv2.0",
    model_card_data=PylateModelCardData(model_name="GerColBERT2"),
)

model.push_to_hub(".....", private=True,  exist_ok=True)



@NohTow NohTow self-assigned this Feb 28, 2025
@sam-hey
Copy link
Contributor Author

sam-hey commented Feb 28, 2025

If you're generally okay with this PR, @NohTow, what do you think about testing this? We could address it by pushing to a private LightOnAI hf repo and then loading from it as a test case. Does that sound like a good idea to you?

At least for main?

@NohTow
Copy link
Collaborator

NohTow commented Feb 28, 2025

Hello,
First, sorry if we are delaying you a bit lately, lot of things on my plate!

I actually did not test pushing to hub the models trained, but the model card created locally during the saving seemed to be overwritten by our template, so the save function looks alright?
If I check the push_to_hub function from ST, it just pushes the folder to the hub, so it should be fine.
Do the files saved locally during your training have the correct model card or not?

I am trying to figure out what's wrong, let me know if I am missing something

@NohTow
Copy link
Collaborator

NohTow commented Feb 28, 2025

Actually I think I understand, the card was working before #87, where I removed the function to not override a function that has no change w.r.t ST and benefits from Tom's changes without doing anything, but there is a regression because it does not call our function anymore.
Could you do the push_to_hub of a folder created after training (possibly with NanoBEIR evaluation) and give me a link just so I double check? Otherwise it seems fine to me
I'll try to have a double check if there is not a prettier solution, but it should be fine for now

@sam-hey
Copy link
Contributor Author

sam-hey commented Feb 28, 2025

No worries about the delay!

I've submitted a PR for ST to improve this, as I share the same sentiment. ^^

I'll keep this PR open as an alternative in case it's rejected. If it's accepted, I'll repurpose this PR to integrate PyLate with my changes.

huggingface/sentence-transformers#3253

@sam-hey
Copy link
Contributor Author

sam-hey commented Mar 11, 2025

@NohTow What do you think about merging this? It should fix the issue. I'll keep track of the pull request on the st library, and if it gets merged, I'll create a new PR to adopt the new approach.

@NohTow
Copy link
Collaborator

NohTow commented Mar 11, 2025

Yes it seems reasonable, it was like that before I tried to better extend ST so it is not a big issue!
Sorry for the delay in merging and do not hesitate to open a new PR if the one on ST gets merged!

@NohTow NohTow merged commit 21e2cb9 into lightonai:main Mar 11, 2025
9 checks passed
@sam-hey sam-hey deleted the model_card branch March 12, 2025 07:12
@sam-hey
Copy link
Contributor Author

sam-hey commented Mar 21, 2025

The changes have been merged into ST! Thanks a lot for your support, @NohTow! Looking forward to ST v4+ so we can bring this update to PyLate.

Have a great weekend!

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.

2 participants