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

Implementing root TMVA for high level analysis #448

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Implementing root TMVA for high level analysis #448

wants to merge 12 commits into from

Conversation

juanangp
Copy link
Member

@juanangp juanangp commented Jun 15, 2023

juanangp Large: 917

Implementing TMVA methods for high level analysis, please check https://root.cern.ch/download/doc/tmva/TMVAUsersGuide.pdf for further details on TMVA.
Summary of changes:

  • New class TRestDataSetTMVA to evaluate different TMVA methods on TRestDatasets note that a signal and a background dataSet must be provided together with different TMVA methods. This class evaluate all the methods provided and generates a root file and a folder with the results.
  • New class TRestDataSetTMVAClassification to performs the classification of a given TRestDataSet using as input the results of the TMVA evaluation methods generated using TRestDataSetTMVA. An output dataset is generated by
    /// definining a new observable with the TMVA method e.g. BDT_score.
  • Added example tmva.rml
  • Minor updates on TRestRun and TRestDataSet

@juanangp juanangp requested review from jgalan, lobis and a team June 15, 2023 20:29
@jgalan
Copy link
Member

jgalan commented Jun 16, 2023

I don't think we should name all the classes using TRestDataSet I think we should assume these classes will use internally TRestDataSet as something natural.

I would only use the DataSet inside the class name when it is clear that we need to differentiate.

For example, inside the sensitivity classes, I plan to have TRestComponent as a base class for TRestDataSetComponent and TRestFormulaComponent.

But it seems to me that TRestDataSetXYZ is over used.

I think discrimination methods usage should be unified by an abstract class TRestDiscrimination. Then TMVA method would be named TRestTMVADiscrimination that uses a TRestDataSet as implemented in TRestDiscrimination.

In my opinion discrimination in our field of research takes such an important role that I would dedicate a directory for all the future discrimination methods. Creating a new directory discrimination.

See issue #13

@juanangp
Copy link
Member Author

I don't think we should name all the classes using TRestDataSet I think we should assume these classes will use internally TRestDataSet as something natural.

Well, I just followed the suggestions done here #392 (comment)
On the other hand, I think we should differenciate between discrimination and data processing, the new TMVA classes are just used to define new observables using TMVA, but the event discrimination is not performed there. I think the TMVA or eithed the LogOdds clasess fits under analysis folder, but we can implement more generic methods for event discrimintation in a different folder.

I don't know how to proceed renaming these classes because is not clear to me the best way to add them to the repository.

@jgalan
Copy link
Member

jgalan commented Jun 23, 2023

I don't think we should name all the classes using TRestDataSet I think we should assume these classes will use internally TRestDataSet as something natural.

Well, I just followed the suggestions done here #392 (comment) On the other hand, I think we should differenciate between discrimination and data processing, the new TMVA classes are just used to define new observables using TMVA, but the event discrimination is not performed there. I think the TMVA or eithed the LogOdds clasess fits under analysis folder, but we can implement more generic methods for event discrimintation in a different folder.

I don't know how to proceed renaming these classes because is not clear to me the best way to add them to the repository.

Ok, perhaps we should discuss this online

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.

None yet

2 participants