add f_net_classifier and f_net_classifier_test #670

ADITYADAS1999 · 2023-01-17T07:57:54Z

Add FNetClassifier #650

Here I add two script at f_net folder.

But we need to improve the f_net_classifier file, because if this model similar to BertClassifier, then the f_net_presets.py file is require that is not present at f_net folder.

@abheesht17 @mattdangerw

mattdangerw · 2023-01-18T03:02:04Z

@ADITYADAS1999 looks like there are some testing failures, could you take a look at those first? You'll probably want an environment in which you can reproduce the failure locally, as described in our contributing guide.

ADITYADAS1999 · 2023-01-18T03:05:54Z

@ADITYADAS1999 looks like there are some testing failures, could you take a look at those first? You'll probably want an environment in which you can reproduce the failure locally, as described in our contributing guide.

okk I test the file locally

This reverts commit 992160d.

This reverts commit fe0f6f4.

ADITYADAS1999 · 2023-01-19T05:20:37Z

@ADITYADAS1999 looks like there are some testing failures, could you take a look at those first? You'll probably want an environment in which you can reproduce the failure locally, as described in our contributing guide.

okk I test the file locally

when I use this commands for windows it show this errors in my pycharm editor

isort --sp "setup.cfg" --sl
black --line-length 80
flake8 --config "setup.cfg" --max-line-length=200

python version 3.7
OS : windows10

abheesht17 · 2023-01-19T08:51:31Z

@ADITYADAS1999, pass the complete path from the root of the repository.

Instead of f_net_classifier.py, pass this: keras_nlp/models/f_net/f_net_classifier.py.

So, your commands will be:

isort --sp "setup.cfg" --sl keras_nlp/models/f_net/f_net_classifier.py
black --line-length 80 keras_nlp/models/f_net/f_net_classifier.py
flake8 --config "setup.cfg" --max-line-length=200 keras_nlp/models/f_net/f_net_classifier.py

isort --sp "setup.cfg" --sl keras_nlp/models/f_net/f_net_classifier_test.py
black --line-length 80 keras_nlp/models/f_net/f_net_classifier_test.py
flake8 --config "setup.cfg" --max-line-length=200 keras_nlp/models/f_net/f_net_classifier_test.py

Hope this solves the issue! :)

ADITYADAS1999 · 2023-01-19T09:10:41Z

@ADITYADAS1999, pass the complete path from the root of the repository.

Instead of f_net_classifier.py, pass this: keras_nlp/models/f_net/f_net_classifier.py.

So, your commands will be:
isort --sp "setup.cfg" --sl keras_nlp/models/f_net/f_net_classifier.py
black --line-length 80 keras_nlp/models/f_net/f_net_classifier.py
flake8 --config "setup.cfg" --max-line-length=200 keras_nlp/models/f_net/f_net_classifier.py

isort --sp "setup.cfg" --sl keras_nlp/models/f_net/f_net_classifier_test.py
black --line-length 80 keras_nlp/models/f_net/f_net_classifier_test.py
flake8 --config "setup.cfg" --max-line-length=200 keras_nlp/models/f_net/f_net_classifier_test.py
Hope this solves the issue! :)

thanks abheesht17, I will try this commands

abheesht17 · 2023-01-19T09:12:57Z

@ADITYADAS1999 - it is recommended that if you are using Windows, use WSL. Makes life very easy! You can follow this guide: https://learn.microsoft.com/en-us/windows/wsl/install.

ADITYADAS1999 · 2023-01-19T14:27:09Z

@ADITYADAS1999 - it is recommended that if you are using Windows, use WSL. Makes life very easy! You can follow this guide: https://learn.microsoft.com/en-us/windows/wsl/install.

installed successfully

can you conform that test commands for linux are correct or not

shell/format.sh
shell/lint.sh

abheesht17 · 2023-01-20T13:26:14Z

@ADITYADAS1999, yep! The commands are correct. Do you have the correct black and flake version though? Please follow the pip install commands given here: https://github.com/keras-team/keras-nlp/blob/master/CONTRIBUTING.md#formatting-code.

ADITYADAS1999 · 2023-01-20T13:30:27Z

@ADITYADAS1999, yep! The commands are correct. Do you have the correct black and flake version though? Please follow the pip install commands given here: https://github.com/keras-team/keras-nlp/blob/master/CONTRIBUTING.md#formatting-code.

yes I install both

ADITYADAS1999 · 2023-01-22T13:32:36Z

hey @mattdangerw can you review this one, If necessary changes are require , All tests are now passed.

mattdangerw

Thank you! And nice work on this!

Overall it is looking good, only major comment is we need to add some more "presets" testing. Commented below. Thanks!

mattdangerw · 2023-01-23T19:42:50Z

keras_nlp/models/f_net/f_net_classifier.py

+
+@keras.utils.register_keras_serializable(package="keras_nlp")
+class FNetClassifier(Task):
+


no newline before docstring

mattdangerw · 2023-01-23T19:45:04Z

keras_nlp/models/f_net/f_net_classifier_test.py

+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Tests for FNET classification model."""


FNET -> FNet (even the paper authors do not style this all caps)

mattdangerw · 2023-01-23T19:46:15Z

keras_nlp/models/f_net/f_net_classifier_test.py

+from keras_nlp.models.f_net.f_net_tokenizer import FNetTokenizer
+
+
+class FNetClassifierTest(tf.test.TestCase, parameterized.TestCase):


You will also need to add some preset tests in f_net_presets_tests.py, you can take a look at #668, which is doing a similar thing for AlBERT.

ADITYADAS1999 · 2023-01-24T04:22:42Z

Thank you! And nice work on this!

Overall it is looking good, only major comment is we need to add some more "presets" testing. Commented below. Thanks!

Thats awesome, made the changes soon

ADITYADAS1999 · 2023-01-24T09:30:36Z

Thank you! And nice work on this!

Overall it is looking good, only major comment is we need to add some more "presets" testing. Commented below. Thanks!

hey @mattdangerw I made necessary changes

This reverts commit 5ccca25.

mattdangerw

@ADITYADAS1999 LGTM.

There are a few test failures currently for the preset tests. Running these are a bit funky... you can specify --run_extra_large flag when running these test, but it will download a lot of data so will be slow (hence the flag)! It helps to only run these tests for a single application at a time.

E.g. you could catch the current error by running

pytest keras_nlp/models/f_net --run_extra_large

But anyway, the issues here our minor, I can just fix these up as I land. Thanks for the contribution!

ADITYADAS1999 · 2023-01-25T03:06:22Z

@ADITYADAS1999 LGTM.

There are a few test failures currently for the preset tests. Running these are a bit funky... you can specify --run_extra_large flag when running these test, but it will download a lot of data so will be slow (hence the flag)! It helps to only run these tests for a single application at a time.

E.g. you could catch the current error by running
pytest keras_nlp/models/f_net --run_extra_large
But anyway, the issues here our minor, I can just fix these up as I land. Thanks for the contribution!

So this command basically tests large files in locally , 👀 I use this for future contribution thanks.

Thanks 👍 @mattdangerw @abheesht17 for fixing and merging the PR 🎉

ADITYADAS1999 added 4 commits January 17, 2023 13:18

add f_net_classifier and f_net_classifier_test

ba4dd4e

Update f_net_classifier.py

ba51c2a

Update f_net_classifier.py

084ce59

remove spaces

b7cc09d

mattdangerw self-requested a review January 18, 2023 02:58

ADITYADAS1999 and others added 6 commits January 18, 2023 09:57

Update f_net_classifier_test.py

fe0f6f4

Update f_net_classifier.py

992160d

Revert "Update f_net_classifier.py"

7cd46df

This reverts commit 992160d.

Revert "Update f_net_classifier_test.py"

ff674c5

This reverts commit fe0f6f4.

remove unnecessary imports

0a746e8

add necessary imports

7bcde56

ADITYADAS1999 added 2 commits January 20, 2023 08:14

Merge branch 'keras-team:master' into new_branch

92699a0

Update f_net_classifier.py

cba52ff

ADITYADAS1999 added 8 commits January 20, 2023 19:38

Delete f_net_classifier.py

c634ae6

Delete f_net_classifier_test.py

583fc00

Add files via upload

06cf94f

Update f_net_classifier_test.py

39f2573

Update f_net_classifier_test.py

4dd5664

Update f_net_classifier.py

ee4c8e9

Update f_net_classifier.py

3a8fa0a

Update f_net_classifier.py

614f8df

ADITYADAS1999 added 10 commits January 21, 2023 13:35

Delete f_net_classifier.py

8a39cd3

Add files via upload

a7c5659

Update f_net_classifier.py

bf44198

Update f_net_classifier.py

5d7b45d

Update f_net_classifier.py

6e1e279

Update f_net_classifier.py

850192d

Update f_net_classifier.py

d33ca41

Delete f_net_classifier_test.py

4844c62

Delete f_net_classifier.py

ec847f3

Add files via upload

7b6225b

mattdangerw requested changes Jan 23, 2023

View reviewed changes

ADITYADAS1999 added 6 commits January 24, 2023 09:59

suggested changes

8d5165d

suggested changes

f159ad2

Update f_net_presets_test.py

9cab7d2

Update f_net_presets_test.py

95db3eb

Delete f_net_presets_test.py

4953fe7

Add files via upload

8a6459d

ADITYADAS1999 and others added 3 commits January 24, 2023 20:08

Update f_net_presets_test.py

5ccca25

Revert "Update f_net_presets_test.py"

1986080

This reverts commit 5ccca25.

Fix presets tests

9380fdd

mattdangerw approved these changes Jan 25, 2023

View reviewed changes

mattdangerw merged commit 73475c6 into keras-team:master Jan 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add f_net_classifier and f_net_classifier_test #670

add f_net_classifier and f_net_classifier_test #670

ADITYADAS1999 commented Jan 17, 2023

mattdangerw commented Jan 18, 2023

ADITYADAS1999 commented Jan 18, 2023

ADITYADAS1999 commented Jan 19, 2023

abheesht17 commented Jan 19, 2023

ADITYADAS1999 commented Jan 19, 2023 •

edited

Loading

abheesht17 commented Jan 19, 2023

ADITYADAS1999 commented Jan 19, 2023 •

edited

Loading

abheesht17 commented Jan 20, 2023

ADITYADAS1999 commented Jan 20, 2023 •

edited

Loading

ADITYADAS1999 commented Jan 22, 2023

mattdangerw left a comment

mattdangerw Jan 23, 2023

mattdangerw Jan 23, 2023

mattdangerw Jan 23, 2023

ADITYADAS1999 commented Jan 24, 2023

ADITYADAS1999 commented Jan 24, 2023

mattdangerw left a comment

ADITYADAS1999 commented Jan 25, 2023


		@keras.utils.register_keras_serializable(package="keras_nlp")
		class FNetClassifier(Task):

		from keras_nlp.models.f_net.f_net_tokenizer import FNetTokenizer


		class FNetClassifierTest(tf.test.TestCase, parameterized.TestCase):

add f_net_classifier and f_net_classifier_test #670

add f_net_classifier and f_net_classifier_test #670

Conversation

ADITYADAS1999 commented Jan 17, 2023

mattdangerw commented Jan 18, 2023

ADITYADAS1999 commented Jan 18, 2023

ADITYADAS1999 commented Jan 19, 2023

abheesht17 commented Jan 19, 2023

ADITYADAS1999 commented Jan 19, 2023 • edited Loading

abheesht17 commented Jan 19, 2023

ADITYADAS1999 commented Jan 19, 2023 • edited Loading

abheesht17 commented Jan 20, 2023

ADITYADAS1999 commented Jan 20, 2023 • edited Loading

ADITYADAS1999 commented Jan 22, 2023

mattdangerw left a comment

Choose a reason for hiding this comment

mattdangerw Jan 23, 2023

Choose a reason for hiding this comment

mattdangerw Jan 23, 2023

Choose a reason for hiding this comment

mattdangerw Jan 23, 2023

Choose a reason for hiding this comment

ADITYADAS1999 commented Jan 24, 2023

ADITYADAS1999 commented Jan 24, 2023

mattdangerw left a comment

Choose a reason for hiding this comment

ADITYADAS1999 commented Jan 25, 2023

ADITYADAS1999 commented Jan 19, 2023 •

edited

Loading

ADITYADAS1999 commented Jan 19, 2023 •

edited

Loading

ADITYADAS1999 commented Jan 20, 2023 •

edited

Loading