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

Bump version to 0.7.0 #1063

Merged
merged 13 commits into from
Mar 26, 2024
Merged

Bump version to 0.7.0 #1063

merged 13 commits into from
Mar 26, 2024

Conversation

irenedea
Copy link
Contributor

@irenedea irenedea commented Mar 26, 2024

Bumps version to 0.7.0 and removes deprecated features.

Removals

  • Remove triton
  • Remove prefixlm
  • Remove llama attention patch
  • Remove z-loss
  • Remove text denoising

Regression tests all passed: llm-foundry-regression-tests-runner-4bPcjd llm-foundry-regression-tests-runner-s1KZPH

.github/workflows/pr-cpu.yaml Outdated Show resolved Hide resolved
.github/workflows/pr-gpu.yaml Outdated Show resolved Hide resolved
.github/workflows/smoketest.yaml Outdated Show resolved Hide resolved
.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved
.github/workflows/code-quality.yaml Outdated Show resolved Hide resolved
@irenedea irenedea marked this pull request as ready for review March 26, 2024 15:51
@irenedea irenedea requested a review from a team as a code owner March 26, 2024 15:51
Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

LGTM, will see if Alex can take a quick skim

@irenedea irenedea enabled auto-merge (squash) March 26, 2024 23:05
@irenedea irenedea merged commit 7f0fdae into main Mar 26, 2024
9 checks passed
Copy link
Contributor

@alextrott16 alextrott16 left a comment

Choose a reason for hiding this comment

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

Minor changes requested. Otherwise LGTM.

RIP denoising.
RIP prefix LM.

is_subseq, batch['bidirectional_mask'][j] ==
1)],
skip_special_tokens=False,
clean_up_tokenization_spaces=True))
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't delete these lines. Instead, find a different way to slice out the context. batch['bidirectional_mask'][j] == 1 was just a convenient way to do that. The context occurs where attention_mask==1 and labels==_HF_IGNORE_INDEX.

tokenizer.decode(batch['input_ids'][
j, batch['bidirectional_mask'][j] == 1],
skip_special_tokens=False,
clean_up_tokenization_spaces=True))
Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment. Don't get rid of this, just do slicing differently.

@@ -55,43 +55,10 @@ def validate_config(cfg: DictConfig):
loaders.append(eval_loader)
for loader in loaders:
if loader.name == 'text':
if cfg.model.name in ['hf_prefix_lm', 'hf_t5']:
if cfg.model.name in ['hf_t5']:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if cfg.model.name in ['hf_t5']:
if cfg.model.name == 'hf_t5':

KuuCi pushed a commit that referenced this pull request Apr 18, 2024
* Bump version

* Remove triton (#1062)

* Remove github action workflows for version bumps

* Fix cpu test issues

* code quality

* Fix gpu tests

* Fix gpu tests nicely

* Remove z-loss (#1064)

* Remove prefix lm and denoising (#1065)

* Remove hf_prefix_lm

* Remove prefix_lm from mpt modeling

* Remove bidirectional mask

* Remove text denoising dataloading

* Remove adapt tokenizer

* Remove llama attention patch (#1066)

* Remove bidirectional mask in tests

* Fix test_hf_config_override with patch
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.

3 participants