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

Improve no-debug option / Improve error messages #194

Closed
frostedoyster opened this issue May 18, 2024 · 3 comments · Fixed by #242
Closed

Improve no-debug option / Improve error messages #194

frostedoyster opened this issue May 18, 2024 · 3 comments · Fixed by #242
Labels
Infrastructure: Logging Related to logging Priority: High Critical issues needing immediate attention.

Comments

@frostedoyster
Copy link
Collaborator

frostedoyster commented May 18, 2024

I'm finding it very difficult to work without the --debug option. When that option isn't on, you can get very cryptic error messages that would be especially unhelpful for the users (and anyone trying to fix the issues). Take this as an example:

training_set:
  systems:
    read_from: qm9_reduced_100.xyz
    length_unit: angstrom
  targets:
    energy:
      key: hello
      unit: eV

where the "hello" key is not present in the .xyz file. This results in:

[2024-05-18 21:03:22][INFO] - This log is also available in 'outputs/2024-05-18/21-03-22/train.log'.
[2024-05-18 21:03:23][INFO] - random seed of this run is 193713243
[2024-05-18 21:03:23][INFO] - Setting up training set
ERROR: 'hello'

which completely hides the error message. Given that some training runs might be very expensive, I think it's a bad idea to have the user re-run it with --debug, especially because all issues with eval only show up at the end of training. I think the easiest solution would be to remove the flag

@frostedoyster frostedoyster changed the title Remove debug Remove debug option May 18, 2024
@frostedoyster frostedoyster changed the title Remove debug option Remove --debug option May 18, 2024
Copy link

you can turn it around to have a --quiet option to turn off output and a --verbose when you want all details

@Luthaf
Copy link
Contributor

Luthaf commented May 21, 2024

On top of whatever changes we make to --debug, we should never produce an error message like ERROR: 'hello'. We can and should strive for more informative messages!

@PicoCentauri
Copy link
Contributor

The only extra thing that --debug does is that the code will print the traceback to line where it broke

https://github.com/lab-cosmo/metatensor-models/blob/3fa7fbfe228bc1be06122c10201b984b3ded5edd/src/metatensor/models/__main__.py#L108-L112

And, --debug will print message in the loglevel DEBUG. Even though I think non of the models currently are using this possibility.

I think for a user this traceback is really useless. If a production software on your machine, like your vscode, produces an error there is no traceback to the typescript blob displayed. Probably just a more or less useful message. This is also what we should aim for.

Useful error message from a which a user knows what to do without reading the traceback call

100% agree with @Luthaf that the one you got is really bad.

Of course you find it annoying to add --debug but you are also a developer and not a user. Besides improving the error message you can always create an alias in your .bashrc.

alias metatens-models="metatensor-models --debug"

@PicoCentauri PicoCentauri changed the title Remove --debug option Improve --debug option Jun 3, 2024
@PicoCentauri PicoCentauri added the Priority: High Critical issues needing immediate attention. label Jun 3, 2024
@PicoCentauri PicoCentauri changed the title Improve --debug option Improve no-debug option / Improve error messages Jun 3, 2024
@PicoCentauri PicoCentauri added Infrastructure: Logging Related to logging and removed infrastructure labels Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure: Logging Related to logging Priority: High Critical issues needing immediate attention.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants