A step that runs seq2seq LMs in inference mode #119

dirkgr · 2021-12-16T19:55:45Z

There is so much stuff in here, it's probably easiest to look at the changelog: https://github.com/allenai/tango/pull/119/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4ed

examples/eval_p3/config.jsonnet

epwalsh · 2022-01-14T17:47:49Z

examples/eval_p3/requirements.txt

@@ -0,0 +1 @@
+rouge-score


You'll need to add this requirement to dev-requirements.txt.

It's helpful for CI so we can install everything with one command. For example, pip install -e [dev,examples]. Speaking of, it would be great to have tests for this example just like we have for the train_gpt2 example.

tango/.github/workflows/ci.yml

Lines 81 to 85 in 8e09b66

- name: GPT2 example

extras: dev,examples,datasets,torch

run: |

cd examples/train_gpt2

pytest -v --color=yes test.py

I think I will switch this to torchmetrics. It's not a new dependency, because it's part of PTL. And as of today, it has Rouge.

87bfc53

But, to make this work, I had to make "examples" into an integration. torchmetrics is installed with lightning, but I want to guarantee the correct version. Since 0.7.0 came out today, it does not yet get installed by default.

Why does "examples" have to be an integration? Why not put this new dependency in dev-requirements.txt along with other dependencies for examples?

tango/dev-requirements.txt

Lines 54 to 57 in 8e09b66

##################################################

###### Extra dev dependencies for examples #######

##################################################

transformers # needed by: examples

I put it there.

epwalsh · 2022-01-14T17:49:40Z

tango/common/logging.py

@@ -114,3 +116,78 @@ def initialize_logging(
        FILE_FRIENDLY_LOGGING = True
        os.environ["FILE_FRIENDLY_LOGGING"] = "true"
        click_logger.disabled = True
+
+
+def logging_tqdm(


How is this different from our Tqdm wrapper with FILE_FRIENDLY_LOGGING on?

You don't have to set an environment variable. It's always on.

It does not redirect stderr in a dodgy way.

Log messages get written to your logger, not the global "tqdm" logger.

It does not depend on implementation details of the original tqdm.

It is less code.

Let me rephrase. What I'm really asking is this: (A) why not implement this using our existing Tqdm wrapper, and (B) why have two separate approaches for file friendly progress bars?

For (A), your 2nd point is still valid, but your approach of implementing a new tqdm from scratch adds more code that looks fairly tricky. We should at least have some test coverage here.

But to emphasize (B) again, it seems like there is a lot of overlap in use-cases here for your logging_tqdm and the existing Tqdm wrapper with FILE_FRIENDLY_LOGGING on. I'd rather go with one or the other. Maybe our file-friendly version of the Tqdm wrapper uses your logging_tqdm code instead of tqdm internals.

I'm happy to write tests for it, but only if we decide to actually use it.

I looked at our current usage of TQDM, and the only feature that logging_tqdm is missing is wrapattr(). There are a few others, but those only apply to rendering visual progress bars, so we don't have to care about them.

Overall I think logging_tqdm() is the superior solution, but there is a lot to do, and the success of Tango won't hinge on the quality of the progress bars. So I'll take this thing out and replace it with FILE_FRIENDLY_LOGGING.

…nto RunWithoutConfig

examples/eval_p3/eval.py

tango/common/util.py

epwalsh · 2022-01-18T22:26:22Z

tango/common/util.py

@@ -153,3 +153,34 @@ def could_be_class_name(name: str) -> bool:

 def _is_valid_python_name(name: str) -> bool:
    return bool(name and name[0].isalpha() and name.isalnum())
+
+
+def threaded_generator(g, queue_size: int = 16):


If you want this in the API docs you can import it in tango/common/__init__.py.

CHANGELOG.md

dirkgr · 2022-01-19T21:25:27Z

I think the last thing in here is the issue with how to select GPUs. I see how you did it in afade92. There, you made the evaluation step completely automatic, while the training step takes the device_count parameter. If we decide to do that here, this PR is done. Sound good @epwalsh?

epwalsh · 2022-01-19T21:44:41Z

There, you made the evaluation step completely automatic, while the training step takes the device_count parameter. If we decide to do that here, this PR is done. Sound good @epwalsh?

Yes, I mean, the only reason the TorchEvalStep doesn't take a device_count parameter at the moment is because it only works on a single device (for now).

dirkgr · 2022-01-19T21:48:14Z

Same is true for this step. But even if it could run on multiple devices, it would not alter the results. We could still put in a device_count parameter, but it would be a SKIP_ID parameter.

dirkgr · 2022-01-19T22:01:30Z

Is this good to go then?

dirkgr · 2022-01-19T22:37:45Z

Ah, I have to call resolve_device() as soon as #120 is merged.

epwalsh

Yeup, this looks good other than the merge conflicts and using resolve_device() ✅

# Conflicts: # CHANGELOG.md # tango/common/logging.py # tango/local_workspace.py

dirkgr added 6 commits December 16, 2021 10:20

Add logging TQDM

2ade4dd

Add threaded generator

4aa6ffd

Adds a step that runs generation

a93373a

More informative error message

8d8256c

Formatting

1dcdd1f

Formatting and other automated tests

58fb7db

dirkgr self-assigned this Dec 16, 2021

dirkgr added 23 commits December 16, 2021 11:55

Merge remote-tracking branch 'origin/main' into RunGeneration

f1756ca

Changelog

c8ec2bf

Added transformers to the requirements

0078778

Adds a text format

a990ae8

Clarifying comment for TextFormat

b96c905

Work correctly with multiple generations per prompt

aa6bf7f

Formatting

6ee8dd8

Work with models that don't have length restrictions

9ddfa90

Needed by some tokenizers

aab2e05

Fix info message

9692bbf

Put tensors on the correct device

5523033

Work with seq2seq style models as well

060a36b

Give the step a version

428b1fd

Fix for autoregressive models

472481d

Don't need these comments anymore

e9c4891

Be more flexible with tokens

5f1bf77

No special support for ctrl

f567e1f

Merge remote-tracking branch 'origin/main' into RunGeneration

459c7d1

Merge branch 'main' into RunGeneration

855ca30

Merge remote-tracking branch 'origin/RunGeneration' into RunGeneration

5c0add2

Inference mode

99fb2e5

Note about transfo special case

c0e3a33

Adds the ability to not use step arguments for the unique id

55ab12b

epwalsh requested changes Jan 14, 2022

View reviewed changes

dirkgr and others added 5 commits January 14, 2022 12:12

Merge branch 'main' into RunGeneration

5d54235

Add some commentary to the P3 example.

15e535a

Merge branch 'RunGeneration' into RunWithoutConfig

61d1147

Don't crash when we don't have a config

16dc3ea

Merge branch 'RunWithoutConfig' of https://github.com/allenai/tango i…

3ff3086

…nto RunWithoutConfig

epwalsh reviewed Jan 18, 2022

View reviewed changes

dirkgr added 7 commits January 18, 2022 17:21

Adds tests for LocalWorkspace, and makes it backwards compatible

2cd8994

Improvemente to threaded_generator()

dcef5b4

Time-based tests are always flaky

3806ac2

Use torchmetrics instead

87bfc53

One more integration

41a7d5e

Actually use the stemmer if required

dc6c2e3

Merge remote-tracking branch 'origin/RunGeneration' into RunGeneration

4cd6096

dirkgr mentioned this pull request Jan 19, 2022

Run without an executor #140

Closed

dirkgr added 3 commits January 19, 2022 10:27

Moved requirements

cc6a238

Fix test for Pytohn 3.7

07cef22

We have only one progress bar solution at a time

c3a743b

Merge branch 'main' into RunGeneration

5ad133f

epwalsh approved these changes Jan 19, 2022

View reviewed changes

dirkgr added 3 commits January 19, 2022 15:23

Merge remote-tracking branch 'origin/main' into RunGeneration

1e17fba

# Conflicts: # CHANGELOG.md # tango/common/logging.py # tango/local_workspace.py

Merge remote-tracking branch 'origin/main' into RunGeneration

0a48ae7

Use Pete's new tools

7ed4ed3

dirkgr merged commit df301ef into main Jan 20, 2022

dirkgr deleted the RunGeneration branch January 20, 2022 01:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A step that runs seq2seq LMs in inference mode #119

A step that runs seq2seq LMs in inference mode #119

dirkgr commented Dec 16, 2021 •

edited

Loading

epwalsh Jan 14, 2022

dirkgr Jan 14, 2022

epwalsh Jan 18, 2022

dirkgr Jan 19, 2022

dirkgr Jan 19, 2022

epwalsh Jan 19, 2022

dirkgr Jan 19, 2022

dirkgr Jan 19, 2022

epwalsh Jan 14, 2022

dirkgr Jan 15, 2022

epwalsh Jan 18, 2022 •

edited

Loading

dirkgr Jan 19, 2022

dirkgr Jan 19, 2022

epwalsh Jan 18, 2022

dirkgr Jan 19, 2022

dirkgr commented Jan 19, 2022

epwalsh commented Jan 19, 2022

dirkgr commented Jan 19, 2022

dirkgr commented Jan 19, 2022

dirkgr commented Jan 19, 2022

epwalsh left a comment

	- name: GPT2 example
	extras: dev,examples,datasets,torch
	run: \|
	cd examples/train_gpt2
	pytest -v --color=yes test.py

	##################################################
	###### Extra dev dependencies for examples #######
	##################################################
	transformers # needed by: examples

		@@ -0,0 +1 @@
		rouge-score

A step that runs seq2seq LMs in inference mode #119

A step that runs seq2seq LMs in inference mode #119

Conversation

dirkgr commented Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epwalsh Jan 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirkgr commented Jan 19, 2022

epwalsh commented Jan 19, 2022

dirkgr commented Jan 19, 2022

dirkgr commented Jan 19, 2022

dirkgr commented Jan 19, 2022

epwalsh left a comment

Choose a reason for hiding this comment

dirkgr commented Dec 16, 2021 •

edited

Loading

epwalsh Jan 18, 2022 •

edited

Loading