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

Falcon: make activation, ffn_hidden_size configurable #30134

Merged
merged 7 commits into from
Apr 11, 2024

Conversation

sshleifer
Copy link
Contributor

@sshleifer sshleifer commented Apr 9, 2024

Defaults unchanged

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature!

Just a comment on adding the new params to the docstring - which should resolve some of the failing tests. For style checks - running make fixup should fix

src/transformers/models/falcon/configuration_falcon.py Outdated Show resolved Hide resolved
@sshleifer
Copy link
Contributor Author

I fixed ruff check examples tests src utils
Dont know how to fix python utils/check_docstrings.py. On my mac it raises

[1]    44425 illegal hardware instruction  python utils/check_docstrings.py

@sshleifer
Copy link
Contributor Author

The docstring traceback is

2024-04-09 19:02:18.387924: W tensorflow/compiler/tf2tensorrt/utils/py_utils.cc:38] TF-TRT Warning: Could not find TensorRT
/home/circleci/transformers/src/transformers/deepspeed.py:23: FutureWarning: transformers.deepspeed module is deprecated and will be removed in a future version. Please import deepspeed modules directly from transformers.integrations
  warnings.warn(
Traceback (most recent call last):
  File "utils/check_docstrings.py", line 1249, in <module>
    check_docstrings(overwrite=args.fix_and_overwrite)
  File "utils/check_docstrings.py", line 1241, in check_docstrings
    raise ValueError(error_message)
ValueError: There was at least one problem when checking docstrings of public objects.
The following objects docstrings do not match their signature. Run `make fix-copies` to fix this. In some cases, this error may be raised incorrectly by the docstring checker. If you think this is the case, you can manually check the docstrings and then add the object name to `OBJECTS_TO_IGNORE` in `utils/check_docstrings.py`.
- FalconConfig
- ```

@amyeroberts
Copy link
Collaborator

amyeroberts commented Apr 10, 2024

I fixed ruff check examples tests src utils Dont know how to fix python utils/check_docstrings.py. On my mac it raises

[1]    44425 illegal hardware instruction  python utils/check_docstrings.py

I really have no idea on this one :/ We had a similar issue opened here #17402. At the time, it was related to TF installations on the mac.

I would try creating a new env and reinstalling with pip install -e .[torch,testing]

@amyeroberts
Copy link
Collaborator

The docstring traceback is

2024-04-09 19:02:18.387924: W tensorflow/compiler/tf2tensorrt/utils/py_utils.cc:38] TF-TRT Warning: Could not find TensorRT
/home/circleci/transformers/src/transformers/deepspeed.py:23: FutureWarning: transformers.deepspeed module is deprecated and will be removed in a future version. Please import deepspeed modules directly from transformers.integrations
  warnings.warn(
Traceback (most recent call last):
  File "utils/check_docstrings.py", line 1249, in <module>
    check_docstrings(overwrite=args.fix_and_overwrite)
  File "utils/check_docstrings.py", line 1241, in check_docstrings
    raise ValueError(error_message)
ValueError: There was at least one problem when checking docstrings of public objects.
The following objects docstrings do not match their signature. Run `make fix-copies` to fix this. In some cases, this error may be raised incorrectly by the docstring checker. If you think this is the case, you can manually check the docstrings and then add the object name to `OBJECTS_TO_IGNORE` in `utils/check_docstrings.py`.
- FalconConfig
- ```

Have you been able to run make fix-copies?

@sshleifer
Copy link
Contributor Author

I think I got it, just ran everything in linux.
Now make fix copies only complains about other models

python utils/check_copies.py --fix_and_overwrite
Detected changes, rewriting src/transformers/models/bigbird_pegasus/modeling_bigbird_pegasus.py.
Detected changes, rewriting src/transformers/models/cohere/modeling_cohere.py.
Detected changes, rewriting src/transformers/models/gemma/modeling_gemma.py.
Detected changes, rewriting tests/models/roc_bert/test_tokenization_roc_bert.py.
python utils/check_table.py --fix_and_overwrite
python utils/check_dummies.py --fix_and_overwrite
python utils/check_doctest_list.py --fix_and_overwrite
python utils/check_task_guides.py --fix_and_overwrite
python utils/check_docstrings.py --fix_and_overwrite

@sshleifer
Copy link
Contributor Author

Do I need another approval?
image

@LysandreJik

@amyeroberts
Copy link
Collaborator

Now make fix copies only complains about other models

Hmmm, that's weird, there shouldn't be any changes to those models as they don't copy from Falcon 🤔 What do you see in the diff from running make fix-copies?

Do I need another approval?

No, this is for running the doctest and is guarded for security reasons. I can run them, but we don't need to for this PR. It takes a long time, so it's better to just skip if not needed

@sshleifer
Copy link
Contributor Author

sshleifer commented Apr 10, 2024

Rebase fixed it. Now it doesnt complain !
Thanks for the help :)
(this is from my fork)
image

@sshleifer
Copy link
Contributor Author

Are we good to merge?

@amyeroberts amyeroberts merged commit edf0935 into huggingface:main Apr 11, 2024
17 checks passed
ArthurZucker pushed a commit that referenced this pull request Apr 22, 2024
* Falcon chg

* delta

* Docstring

* Fix import block

* doc

* fix and overwrite
itazap pushed a commit that referenced this pull request May 14, 2024
* Falcon chg

* delta

* Docstring

* Fix import block

* doc

* fix and overwrite
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