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

Parameter/Component tables are too wide #358

Closed
veprbl opened this issue Sep 11, 2024 · 6 comments
Closed

Parameter/Component tables are too wide #358

veprbl opened this issue Sep 11, 2024 · 6 comments

Comments

@veprbl
Copy link
Contributor

veprbl commented Sep 11, 2024

These tables are unreadable in EICrecon because C++'s typeinfo's can be very long.

Can we also remove those from JApplication::Run()?

@nathanwbrei
Copy link
Collaborator

We sure can

@DraTeots
Copy link
Collaborator

Those actually are very handy for some debug and new app build cases. EICrecon has more or less static number of components/flags from run to run and established code, so they are not useful. DAQ and BeamTest apps has plugins set depending on runs and vary a lot, so the table helps sometimes to understand what is happening. So I would leave a flag to show them. Indeed they might be difficult to read especially if one uses C++20 modules. Maybe make it less "table-y"?

@nathanwbrei
Copy link
Collaborator

Yeah, there's definitely a conflict between "what makes sense for a user running in a terminal" vs "what makes sense for debugging". I know that @DraTeots has been clamoring for JSON structured output for a long time! Here's some semi-organized thoughts that have been bouncing around in my head:

  • Ideally users would be able to control the output of their eicrecon style programs exactly, i.e. not sending all logging to std::cout to begin with

  • Right now most status-y things that we log we can extract directly through the API, e.g. from JComponentSummary, although I'd like to clean it up a bit before committing to supporting it officially. So in principle we could redirect all logging to a text file and have eicrecon print exactly what its users want.

  • I'm happy with having the fundamental unit of logging be individual components, e.g. OmniFactories. We are already doing this using parameters like so: my_component:loglevel=debug and I think this makes a lot of sense to users, at least compared to the other options! It also lets different loggers use the same configuration, in case we really do go down the issue Feature request: JLogger optionally redirects to spdlog #250 route.

  • Right now JANA configures loggers at the className level, which is really bad for usability, even for me, because you have to know which class would produce the log message you expect. I'm leaning more and more towards having there be exactly one "logging group" for all JANA-internal loggers, so you can dial the verbosity of JANA's internal logging up and down with a single parameter. This means more verbose output, but again, if you aren't using the logger as your primary user interface, you're better off collecting more information than you need and filtering it later.

  • I have mixed feelings about putting JSON in the log output. If the goal is to create a clean Python interface, I think adding bindings for JComponentSummary, JPerfSummary, etc, is a better way to go. (Although we already have JSON generators for a lot of our data structures, both for janacontrol plugin and for JInspector.) It would be easy to add a parameter to switch between table and JSON output, if that's really what we want. I think Julia does an exceptionally good job with its builtin logger (https://docs.julialang.org/en/v1/stdlib/Logging/) at threading the needle between being pleasant to look at and accurately (and automatically!) displaying nontrivial data structures. However, recreating that in C++ is probably not even possible, at least without very thoughtful Cling introspection.

@nathanwbrei
Copy link
Collaborator

I think one particularly tricky thing to get right is actually the status ticker. In principle, JANA can run without any supervisor thread, but in practice we want to do things like detect timeouts, so having a built-in supervisor thread is the way to go. The supervisor thread is the obvious choice for printing the ticker. However, if the built-in supervisor thread isn't responsible for printing the ticker, there has to something at the EICrecon level that is polling the JApplication in order to print the ticker. I don't want EICrecon to have to implement another supervisor thread! As it turns out, a bunch of JANA plugins also want to create the own thread that occasionally interacts with the JApplication, so I'm tempted to create a new kind of component so that users can add callbacks that get run by the supervisor thread, instead of having n threads floating around. One of which could be an eicrecon-specific status ticker...

@nathanwbrei
Copy link
Collaborator

Also, parameters are shown at level WARN and components are shown at level INFO, so if you want to suppress the collection tables (and everything else that is non-essential), simply run with -Pjana:loglevel=warn. WARN now includes the signal handler instructions, the JANA version and install prefix, the loaded plugin paths, parameters, ticker, and final event count+rate.

@veprbl
Copy link
Contributor Author

veprbl commented Sep 16, 2024

Why component table printout has to be hardcoded in JApplication? Can't you move the call to your CLI.

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

No branches or pull requests

3 participants