FairRootGroup / FairRoot

C++ simulation, reconstruction and analysis framework for particle physics experiments
http://fairroot.gsi.de
Other
57 stars 96 forks source link

Error message for empty container in FairRunTimeDB #1541

Open TobiasStockmanns opened 3 months ago

TobiasStockmanns commented 3 months ago

FairRunTimeDB generates an error message

-I- FairRunTimeDB::InitContainer() PndEmcGeoPar
[ERROR] init() PndEmcGeoPar not initialized

when a parameter container is not filled with existing data from one of the two input possibilities (either an ASCII or root par file).

In some cases these parameters are generated inside the program code and then stored into the runtime DB creating this error message even though it is not an error.

The desired behaviour would be that the message is marked as [INFO] and not as [ERROR]. The text could look like: [INFO] init() PndEmcGeoPar does not contain any data.

ChristianTackeGSI commented 2 months ago

I added #1551 to improve the logging of this. (I had this patch around for a while anyway. So a good opportunity to turn it into a PR).

@karabowi and I had a quick chat two days ago. And we agreed, that this error is generally a good thing. And users who know what they're doing (@TobiasStockmanns that's you) should be able to turn a knob and disable this error in a fine granular way.

This error happens in FairParSet::init(). This member function is virtual (whether is was a good design or not… other topic).

Options:

  1. So the most quickest (and probably a bit dirty) way: Override this function in PndEmcGeoPar and implement it like you want, maybe by copying anything interesting from the original (possibly dropping all the init(io) logic from the original?).

  2. We split up the current init() into more peaces and make them available as helper functions that don't throw errors but have the "important" logic and let them be re-used in our main, provided init(). So that you can re-arrange those helpers as needed and implement different error handling.

  3. We add another knob (probably something like virtual bool NotBeingAbleToLoadFromIoIsOk() { return false; } – yes, this long, you're not supposed to have an easy live disabling errors) and call it from the current init().

TBH, I have no particular preference.

ChristianTackeGSI commented 2 months ago

Next possible option:

FairParSet seems to have a "static" mode:

https://github.com/FairRootGroup/FairRoot/blob/32bb52f41d5945acf128dcb3c712fa91fe2b5103/fairroot/parbase/FairParSet.h#L63-L64

This completely disables running init() (and thus avoid the error completely by not even trying to initialize things):

https://github.com/FairRootGroup/FairRoot/blob/32bb52f41d5945acf128dcb3c712fa91fe2b5103/fairroot/parbase/FairRuntimeDb.cxx#L545-L547

But it also disables resetInputVersions:

https://github.com/FairRootGroup/FairRoot/blob/32bb52f41d5945acf128dcb3c712fa91fe2b5103/fairroot/parbase/FairRuntimeDb.cxx#L681-L683

If I understood @karabowi correctly in a private chat, then this "static" feature is meant for something else anyway?

ChristianTackeGSI commented 2 months ago

After having thought about option (1) above, I consider it quite useful.

There are some options:

struct PndEmcGeoPar : public FairParGenericSet {
  bool do_not_call_init_at_all{false};  //!
  bool ignore_errors_from_init{false};  //!

  bool init() override {
    if (do_not_call_init_at_all) {
      return true;
    }
    return FairParGenericSet::init();
// or:
    bool retval = FairParGenericSet::init();
    if (ignore_errors_from_init && !retval) {
      LOG(info) << "The previous error is just informational, please ignore\n"
      return true;
    }
    return retval;
  }
}
fuhlig1 commented 1 month ago

@karabowi,

could you please have a look at the issue since you already discussed with @TobiasStockmanns .

karabowi commented 1 month ago

The issue has been solved in PR #1551. @TobiasStockmanns , can you check the solution?

ChristianTackeGSI commented 1 month ago

1551 improves the situation by moving the error out of init() to the caller of init(). So if it returns true (by overriding it), then no error will be printed. Maybe a warning is printed for init(io) return values.

The above mentioned override variants should still work on older versions, just their output might be a bit confusing (that's why I put the "The previous error is just informational, please ignore" there).