brentyi / tyro

CLI interfaces & config objects, from types
https://brentyi.github.io/tyro
MIT License
467 stars 23 forks source link

can haz __version__ ? #121

Closed pwais closed 7 months ago

pwais commented 7 months ago

i ran into a really confusing backtrace using tyro / nerfstudio and so i wanted to see if i actually had the expected version of tyro installed. but i can't even tell what version i have of tyro because tyro has no __version__

Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tyro
>>> tyro.__version__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'tyro' has no attribute '__version__'
>>> dir(tyro)
['MISSING', 'TYPE_CHECKING', 'UnsupportedTypeAnnotationError', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '_argparse_formatter', '_arguments', '_calling', '_cli', '_deprecated', '_docstrings', '_fields', '_instantiators', '_parsers', '_resolver', '_singleton', '_strings', '_subcommand_matching', '_typing', '_unsafe_cache', 'cli', 'conf', 'extras', 'from_yaml', 'parse', 'to_yaml']
pwais commented 7 months ago

i downgraded to 0.5.10 and now stuff works (but of course still no __version__ )

fwiw the nerfstudio thing was like:

Traceback (most recent call last):
  File "/usr/local/bin/ns-train", line 8, in <module>
    sys.exit(entrypoint())
  File "/opt/nerfstudio/scripts/train.py", line 263, in entrypoint
    tyro.cli(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_cli.py", line 187, in cli
    output = _cli_impl(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_cli.py", line 454, in _cli_impl
    out, consumed_keywords = _calling.call_from_args(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_calling.py", line 157, in call_from_args
    value, consumed_keywords_child = call_from_args(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_calling.py", line 122, in call_from_args
    value, consumed_keywords_child = call_from_args(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_calling.py", line 122, in call_from_args
    value, consumed_keywords_child = call_from_args(
  File "/usr/local/lib/python3.10/dist-packages/tyro/_calling.py", line 247, in call_from_args
    return unwrapped_f(*positional_args, **kwargs), consumed_keywords  # type: ignore
TypeError: DataManagerConfig.__init__() got an unexpected keyword argument 'dataparser'

except the model i was ns-training was very much not using DataManagerConfig and tyro doesn't help you here figure out how method_configs.py config garden got out of sync with tyro cli args :( too much indirection.

brentyi commented 7 months ago

Sure, including __version__ seems reasonable. (although for your purposes does pip freeze | grep tyro not work?)

For your error: do you have reproduction instructions for this? I'd be interested in improving the error message if this is a usage error, or of course to fix any bugs if it's a bug.

pwais commented 7 months ago

@brentyi im between nerfstudio versions but my main feedback on the traceback is:

brentyi commented 7 months ago

Yeah, that makes sense. I'll have the __version__ tag in by the next release.

The error context is a little bit harder, especially since I currently can't reproduce the error to verify that the message is actually helpful, but I can at least think about it...

pwais commented 7 months ago

@brentyi FWIW i've run into basically the same tryo-nerfstudio traceback like above like 100 times 😬 try using the nerfstudio template and insert an oops in the config bits e.g. use the wrong superclass, forget to declare a field, etc ...

another good one that breaks often for me is ns-train method --data=data ... In particular it took me like half an hour to figure out how to do --pipeline.model.collider-params near_plane 0.1 far_plane 10 (the sub-args kept going wrong place?)

and also the calling the datamanager broke recently, like this no longer works or i can't figure out how to make it work:

ns-train \
      nerfacto \
        --data={ns_input} \
        --output-dir={ns_out} \
        --machine.num-gpus=1 \
        --optimizers.fields.optimizer.lr=0.02 \
        --pipeline.datamanager.train-num-rays-per-batch=3200 \
        --pipeline.datamanager.eval-num-images-to-sample-from=1 \
        --logging.local-writer.max-log-size=0 \
        --vis=tensorboard \
      nerfstudio-data \
        --downscale-factor=1

Anyways this is a long-winded request for tyro to somehow communicate the state after parsing but before instantiating. Not an easy problem! I've had mixed luck with simple-parsing and separately omegaconf ... i have favored using simple-parsing recently.

thanks for __version__ !!

brentyi commented 7 months ago

Yeah, I also think simple-parsing is great!

For the TypeError: DataManagerConfig.__init__() got an unexpected keyword argument 'dataparser': I tracked down an old commit of nerfstudio where I was able to reproduce this. And did a 0.7.1 release that should have the bug fixed (removed one line of code) + the _version__ string in there. 🙂

If you run into the issue again on 0.7.1 I'd appreciate a ping.

I'll think about the state communication thing. My initial feeling is that it's pretty hard to do for the general case.