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

Filter auto_transformer kwargs based on forward signature #3329

Merged
merged 2 commits into from
Apr 7, 2023
Merged

Conversation

tgaddair
Copy link
Collaborator

@tgaddair tgaddair commented Apr 7, 2023

Fixes #3328.

@tgaddair tgaddair added bug Something isn't working release-0.7 Needs cherry-pick into 0.7 release branch labels Apr 7, 2023
Copy link
Contributor

@jeffkinnison jeffkinnison 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 some nits and a quick question.

ludwig/encoders/text_encoders.py Show resolved Hide resolved
tests/ludwig/encoders/test_text_encoders.py Show resolved Hide resolved
ludwig/encoders/text_encoders.py Show resolved Hide resolved
"hf-internal-testing/tiny-random-GPTJModel",
],
)
def test_hf_ludwig_model_auto_transformers(tmpdir, csv_filename, pretrained_model_name_or_path):
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 mark this with @pytest.mark.slow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this should only take 30s, so I would want to run it on every commit if we can get away with it.

Comment on lines +174 to +205
input_features = [
text_feature(
preprocessing={
"max_sequence_length": 10,
},
encoder={
"vocab_size": 30,
"min_len": 1,
"type": "auto_transformer",
"pretrained_model_name_or_path": pretrained_model_name_or_path,
"use_pretrained": True,
},
)
]
output_features = [category_feature(decoder={"vocab_size": 2})]
rel_path = generate_data(input_features, output_features, csv_filename)

config = {
"input_features": input_features,
"output_features": output_features,
TRAINER: {"train_steps": 1},
}
model = LudwigModel(config=config, backend=LocalTestBackend())

# Validates that the defaults associated with the encoder are compatible with Ludwig training.
with mock.patch(
"ludwig.encoders.text_encoders.load_pretrained_hf_model_with_hub_fallback",
side_effect=_load_pretrained_hf_model_no_weights,
):
model.train(dataset=rel_path, output_directory=tmpdir)


Copy link
Contributor

Choose a reason for hiding this comment

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

Code here is a duplicate of that in test_hf_ludwig_model_reduce_options and test_hf_ludwig_model_e2e. Perhaps separate into a separate function and reuse.

@tgaddair tgaddair merged commit 46d08c7 into master Apr 7, 2023
6 of 7 checks passed
@tgaddair tgaddair deleted the fix-bloom branch April 7, 2023 19:50
@pushkarraj
Copy link

Hey guys, I am working in the domain of large language models. Thanks for fixing this issue. I would love to contribute in the open source. It would be a matter of pride to be the part of this.

@pushkarraj
Copy link

Also this issue is resolved.after trying the latest code I got the following error:

AttributeError: 'BloomModel' object has no attribute 'n_head'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working release-0.7 Needs cherry-pick into 0.7 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: forward() got an unexpected keyword argument 'token_type_ids'
5 participants