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

Error message for empty container in FairRunTimeDB #1541

Open
TobiasStockmanns opened this issue May 27, 2024 · 6 comments
Open

Error message for empty container in FairRunTimeDB #1541

TobiasStockmanns opened this issue May 27, 2024 · 6 comments
Assignees
Labels
bug need-feedback:submitter waiting for feedback from issue creator

Comments

@TobiasStockmanns
Copy link
Contributor

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.

karabowi added a commit to karabowi/FairRoot that referenced this issue Jun 6, 2024
In rare cases, when parameteres deriving from `FairParSet`
are not present in the input parameter file and are meant
to be created during the run, `FairParSet::init()` throws
an error message.

By using the newly introduced setter `setCreationMode`
this message is suppressed.

This allows solution to issue FairRootGroup#1541.
@ChristianTackeGSI
Copy link
Member

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
Copy link
Member

Next possible option:

FairParSet seems to have a "static" mode:

void setStatic(Bool_t flag = kTRUE) { status = flag; }
Bool_t isStatic() { return status; }

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

if (!cont->isStatic()) {
rc = cont->init() && rc;
}

But it also disables resetInputVersions:

if (!cont->isStatic()) {
cont->resetInputVersions();
}

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

@ChristianTackeGSI
Copy link
Member

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
Copy link
Member

fuhlig1 commented Jul 5, 2024

@karabowi,

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

@karabowi
Copy link
Collaborator

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

@ChristianTackeGSI
Copy link
Member

#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).

@fuhlig1 fuhlig1 added bug need-feedback:submitter waiting for feedback from issue creator labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug need-feedback:submitter waiting for feedback from issue creator
Projects
None yet
Development

No branches or pull requests

4 participants