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

Fix bug and change interface of getMaxWeight and move track/cluster encoding parts to MarlinUtil #163

Merged
merged 4 commits into from
Feb 6, 2023

Conversation

dudarboh
Copy link
Member

@dudarboh dudarboh commented Feb 3, 2023

Make getMaxWeight functionality templated that allows passing any comparator. Moving track/caluster weight encodings functionality into MarlinUtil

Added with LCIO v02-18 (https://github.com/iLCSoft/LCIO/releases/tag/v02-18) utility methods to extract the highest weight for LCRelationNavigator are not working properly.

  1. getMaxWeightIt() must return max_element in every if branch, otherwise passed "weight" parameter is just ignored
    if (weightType == "track") {
    std::max_element(weights.begin(), weights.end(), [](float a, float b) {
    return (int(a) % 10000) / 1000. < (int(b) % 10000) / 1000.;
    });
    } else if (weightType == "cluster") {
    std::max_element(weights.begin(), weights.end(), [](float a, float b) {
    return (int(a) / 10000) / 1000. < (int(b) / 10000) /1000.;
    });
    }
    return std::max_element(weights.begin(), weights.end());
  2. Every new function that uses std::string& weightType as an argument, must have an empty string as a default.

We have discussed with @tmadlener that it would be nice to keep the functionality as general as possible in LCIO and have track/cluster specific weight encoding parts in the MarlinUtil, as these encodings are defined only to some specific LCRelation collections which are created by MarlinReco.

This PR fixes bugs described above and changes the interface to be more generic allowing an arbitrary comparator function as an argument instead of previously string parameter.

Two specific comparator function for track and cluster weights will be added to the MarlinUtil soon.

BEGINRELEASENOTES

  • getRelatedTo(From)MaxWeightObject() and getRelatedTo(From)MaxWeight() now accept generic decode function of float(float) signature as a second argument, which specifies how to decode the weight. Default option is identity function (just compares weights as they are).
  • Helper functions to decode and encode "track"/"cluster" specific weights from PFO-MCParticle LCRelation collection are added to MarlinUtil in MarlinUtil#36.

ENDRELEASENOTES

…parator. Moving track/caluster weight encodings functionality into MarlinUtil
Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor comments / questions (see below).

Maybe one thing that should be added to the docstrings, is the requirements for the CompareF, i.e. the signature is bool(float, float) and that the return value should be true if the first argument is "less" than the second.

src/cpp/include/UTIL/LCRelationNavigator.h Outdated Show resolved Hide resolved
src/cpp/include/UTIL/LCRelationNavigator.h Outdated Show resolved Hide resolved
…comparator. Return already decoded max weights, instead of encoded
@tmadlener tmadlener merged commit be0070a into iLCSoft:master Feb 6, 2023
@dudarboh dudarboh deleted the hotfix_nav branch June 8, 2023 13:39
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

Successfully merging this pull request may close these issues.

2 participants