Skip to content

Conversation

@iseeyuan
Copy link
Contributor

Summary

  1. Add structure to the complicated args
  2. Convert args to DictConfig, to decouple the cli args
  3. Pass needed sub configs to functions (instead of args)

Test plan

python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64

@pytorch-bot
Copy link

pytorch-bot bot commented Mar 20, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9450

Note: Links to docs will display an error until the docs builds have been completed.

❌ 4 New Failures

As of commit bccc2e3 with merge base 76ae537 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@iseeyuan iseeyuan linked an issue Mar 20, 2025 that may be closed by this pull request
@iseeyuan
Copy link
Contributor Author

@pytorchbot label "topic: not user facing"

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 20, 2025
@facebook-github-bot
Copy link
Contributor

@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@iseeyuan iseeyuan force-pushed the dictconfig branch 2 times, most recently from 6df85c1 to 311e925 Compare March 21, 2025 02:31
@facebook-github-bot
Copy link
Contributor

@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment on lines +299 to +305
config.eval = OmegaConf.create()
config.eval.tasks = args.tasks
config.eval.limit = args.limit
config.eval.num_fewshot = args.num_fewshot
config.eval.pte = args.pte
config.eval.tokenizer_bin = args.tokenizer_bin
config.eval.output_eager_checkpoint_file = args.output_eager_checkpoint_file
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry is there a definition of the config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is just the first step: to convert args to config to unblock internal work, where the configs can be used standalone, without cli args and yaml file. More context in #9449

@facebook-github-bot
Copy link
Contributor

@iseeyuan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

"output_dir": args.output_dir,
"checkpoint": args.checkpoint,
"checkpoint_dir": args.checkpoint_dir,
"tokenizer_path": args.tokenizer_path,
Copy link
Contributor

@jackzhxng jackzhxng Mar 24, 2025

Choose a reason for hiding this comment

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

This is only needed during export for quant calibration, should go in args_dict["calibration"]. Or since these args are also used in different runners, I'd prefer to have args_dict["tokenizer"], we can combine it with tokenizer_config_path which the eager runner uses

args_dict["kv_cache"] = {
"use_kv_cache": args.use_kv_cache,
"quantize_kv_cache": args.quantize_kv_cache,
"use_sdpa_with_kv_cache": args.use_sdpa_with_kv_cache,
Copy link
Contributor

Choose a reason for hiding this comment

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

This arg is a bit poorly named now that the custom sdpa op is now decoupled from the kv cache, should move this to arcs_dict["misc"]. We should rename this arg @kimishpatel

facebook-github-bot pushed a commit that referenced this pull request Mar 27, 2025
Summary:
1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)


Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Reviewed By: larryliu0820

Differential Revision: D71557301

Pulled By: jackzhxng
jackzhxng pushed a commit that referenced this pull request Mar 28, 2025
Summary:
Pull Request resolved: #9717

1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)

Pull Request resolved: #9450

Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Reviewed By: larryliu0820

Differential Revision: D71557301

Pulled By: jackzhxng
facebook-github-bot pushed a commit that referenced this pull request Mar 30, 2025
Summary:
1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)


Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Pulled By:
jackzhxng

jackzhxng

Differential Revision: D71557301
facebook-github-bot pushed a commit that referenced this pull request Mar 31, 2025
Summary:
1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)


Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Pulled By:
jackzhxng

jackzhxng

Differential Revision: D71557301
facebook-github-bot pushed a commit that referenced this pull request Mar 31, 2025
Summary:
1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)


Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Pulled By:
jackzhxng

jackzhxng

Differential Revision: D71557301
facebook-github-bot pushed a commit that referenced this pull request Apr 1, 2025
Summary:
1. Add structure to the complicated args
2. Convert args to DictConfig, to decouple the cli args
3. Pass needed sub configs to functions (instead of args)


Test Plan:
```
python3 -m examples.models.llama.export_llama -c stories110M.pt -p params.json -d fp32 -n tinyllama_xnnpack+custom_fp32_main.pte -kv -X --xnnpack-extended-ops -qmode 8da4w -G 128 --use_sdpa_with_kv_cache --output-dir tmp -E 8,64
```

Reviewed By: larryliu0820

Pulled By:
jackzhxng

jackzhxng

Differential Revision: D71557301
@github-actions
Copy link

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the stale PRs inactive for over 60 days label Aug 31, 2025
@github-actions
Copy link

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@jackzhxng jackzhxng closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. stale PRs inactive for over 60 days topic: not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants