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

Implemented Roberta Model #65

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

Conversation

Mr-Niraj-Kulkarni
Copy link

No description provided.

@Mr-Niraj-Kulkarni
Copy link
Author

@juliagsy I am ready for review

Copy link
Contributor

@juliagsy juliagsy left a comment

Choose a reason for hiding this comment

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

Hey! Thanks for the great work so far! On top of the comments, please also populate the __init__.py files so that imports are working correctly, thanks!

ivy_models_tests/roberta/test_roberta.py Outdated Show resolved Hide resolved
ivy_models_tests/roberta/test_roberta.py Outdated Show resolved Hide resolved
@Mr-Niraj-Kulkarni
Copy link
Author

@juliagsy I have implemented the required changes and completed the testing

use_cache=None,
output_attentions=None,
):
if input_ids[:, 0].sum().item() != 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

should be to_scalar instead of item

import pytest
import numpy as np
from ivy_models import roberta_base

Copy link
Contributor

Choose a reason for hiding this comment

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

could you please quickly ref test_alexnet and make the model object init happen once only for saving ci resources, thanks!

if load_weights:
ref_logits_path = os.path.join(this_dir, "roberta_pooled_output.npy")
ref_logits = np.load(ref_logits_path)
assert np.allclose(ref_logits, ivy.to_numpy(logits), rtol=0.005, atol=0.005)
Copy link
Contributor

Choose a reason for hiding this comment

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

the value tests are unfortunately not passing when I checked, could you please verify this again?
also, could you please do me a favour and update the load_transformers_weights helper to not use copy.deepcopy and do old_mapping = model.v directly? it's somewhere line 174 in the weights_helpers.py file, thanks!

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