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

Discard init_args after class_path change causes error #205

Closed
samvanstroud opened this issue Dec 5, 2022 · 4 comments · Fixed by #212
Closed

Discard init_args after class_path change causes error #205

samvanstroud opened this issue Dec 5, 2022 · 4 comments · Fixed by #212
Labels
bug Something isn't working

Comments

@samvanstroud
Copy link

🐛 Bug report

When composing configs, it appears that overwritten class paths do not properly drop the old init_args namespace, causing an error when the new class does not have the same init_args.

Here is the error message:

$ main fit --config config_1.yaml --config configs_2.yaml
/unix/atlastracking/svanstroud/miniconda3/envs/salt/lib/python3.10/site-packages/jsonargparse/typehints.py:909: UserWarning: Due to class_path change from 'models.GATv2Attention' to 'models.ScaledDotProductAttention', discarding init_args: {'num_heads': 8, 'head_dim': 16, 'activation': Namespace(class_path='torch.nn.SiLU')}.
  warnings.warn(
usage: main [-h] [-c CONFIG] [--print_config[=flags]] {fit,validate,test,predict,tune} ...
main: error: Parser key "model": Problem with given class_path 'models.Tagger':
    - Parser key "gnn": Does not validate against any of the Union subtypes
      Subtypes: (<class 'torch.nn.modules.module.Module'>, <class 'NoneType'>)
      Errors:
        - Problem with given class_path 'models.Transformer':
          - Parser key "attention": Problem with given class_path 'models.ScaledDotProductAttention':
              - 'Configuration check failed :: No action for destination key "num_heads" to check its value.'
        - Expected a <class 'NoneType'>
      Given value type: <class 'jsonargparse.namespace.Namespace'>
      Given value: Namespace(class_path='models.Transformer', init_args=Namespace(embd_dim=128, num_heads=8, num_layers=8, attention=Namespace(class_path='models.ScaledDotProductAttention', init_args=Namespace(num_heads=8, head_dim=16, activation=Namespace(class_path='torch.nn.SiLU'))), activation='SiLU', residual=True, norm_layer='LayerNorm', dropout=0.1, out_proj=False))

To reproduce

Expected behavior

The overwriting class should not be required to have the same init_args as the overwritten class.
It would also be nice to have an option to suppress the warning about overwritten args.

Environment

  • jsonargparse 4.18.0
  • Python 3.10.6
@samvanstroud samvanstroud added the bug Something isn't working label Dec 5, 2022
@speediedan
Copy link
Contributor

@samvanstroud Thanks for posting this issue! (and to @mauvilsa et al. for this valuable package!). As I've observed it with Fine-Tuning Scheduler as well, I'm pinning finetuning-scheduler's jsonargparse dependency to >=4.15.2, <4.18.0 until it's resolved.

@samvanstroud
Copy link
Author

samvanstroud commented Dec 12, 2022

@mauvilsa is this related to Lightning-AI/pytorch-lightning#15796? I guess I was surprised that it was not fixed by #199, but I was composing yaml files so perhaps another fix is needed for this case?

@mauvilsa
Copy link
Member

Actually what happened is that this is a regression and was caused by #199. The original issue Lightning-AI/pytorch-lightning#15796 is about discarding init_args when using individual command line arguments, not configs. It wasn't noticed in #199 because there is no unit test for the discard from config files. I will fix this and add a unit tests to prevent future regressions.

@mauvilsa
Copy link
Member

Created a pull request hopefully fixing the issue. Please test it out, e.g.

pip install "jsonargparse @ https://github.com/omni-us/jsonargparse/zipball/issue-205-discard-init-args"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants