search

SBNSoftware / sbnana

3 stars 14 forks source link

Feature/bckhouse_cafana_weight_systs #16

Closed cjbacchus closed 3 years ago

cjbacchus commented 3 years ago

Enable use of weight systematics within CAFAna. Reactivates some code that was taken out of the build when this originally got broken in the transition to CAFs.

wesketchum commented 3 years ago

also, files look ok to me, but content may be better reviewed by someone else? @cjbackhouse any recs? @mastbaum @FernandaPsihas @PetrilloAtWork ?

cjbacchus commented 3 years ago

But VLAs are so useful :(

This is kind of a classic sbnana case where it's not clear how it can be usefully reviewed, beyond basic style and build checks (thanks!).

PetrilloAtWork commented 3 years ago

This is kind of a classic sbnana case where it's not clear how it can be usefully reviewed, beyond basic style and build checks (thanks!).

I, for example, wouldn't know where to start from.

For my education: why the choice of abort()? (and why error messages are written to std::cout?)

cjbacchus commented 3 years ago

The aborts are in places where it's impossible to continue sensibly - we can't do the analysis the user wanted. It's much better to stop and explain why than it is to continue with some different analysis the user didn't request, or to crash later.

If the question is why abort vs throw, this way I get to print the error of my choice and then quit. With an exception, the only printout you tend to get is "unhandled exception". Then you need to use a debugger to find out what happened, which is a pain because all sorts of bad code throws and catches exceptions needlessly. And then in the end all you have is the line number anyway. There's no sensible way to catch this exception apart from to print some error and then abort, which is what we are already doing.

cout vs cerr is maybe lazy, but I would say that the distinction only really makes sense for programs whose output you expect to pipe somewhere. Here the user is just going to be reading it off a screen or a log file anyway. Also, I really hate the thing that many batch systems do where they give you your output and error streams separately, so you can't understand the sequence of events.

miquelnebot commented 3 years ago

Requested changes are done, is this good to go @cjbackhouse ?

cjbacchus commented 3 years ago

Requested changes are done, is this good to go @cjbackhouse ?

Yep!