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

Nkx111 patch 4 #244

Merged
merged 28 commits into from
Jul 12, 2022
Merged

Nkx111 patch 4 #244

merged 28 commits into from
Jul 12, 2022

Conversation

nkx111
Copy link
Member

@nkx111 nkx111 commented Jun 12, 2022

nkx111 Medium: 166

Some bug fix and new features:

  1. Fix bugs when opening files with unknown event types(e.g., v2.2 file). Previsuly in this case the input event will be initialized with data members, but the method cannot be called, and causes seg.fault.
  2. added method: TRestEventProcess::GetParallelDataMembers()
  3. restRoot macro loading now also working on windows.
  4. added validation pipeline to test open v2.2 files.
  5. fixed Console::kbhit() on windows.

source/bin/restRoot.cxx Outdated Show resolved Hide resolved
@nkx111 nkx111 marked this pull request as ready for review June 26, 2022 10:30
@nkx111 nkx111 requested review from lobis and jgalan June 29, 2022 01:00
.gitlab-ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@jgalan jgalan left a comment

Choose a reason for hiding this comment

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

Hi @nkx, perhaps we could create a dedicated data repository, where we place data files such as the one you are adding v2.2.30_hits.root?

This would be something similar to https://github.com/rest-for-physics/axionlib-data/tree/36629372bcffddd6f95eb12a565c1b15475a64be

That we later make http accessible, and use at the pipelines using wget.

@jgalan
Copy link
Member

jgalan commented Jun 29, 2022

What the GetParallelDataMembers is doing?

/// for (auto h : hists) { fHist->Add(h); }
///
template <class T>
std::vector<T> GetParallelDataMembers(T* member_of_process) {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this template used?

@@ -229,4 +259,9 @@ class TRestEventProcess : public TRestMetadata {

ClassDefOverride(TRestEventProcess, 3); // Base class for a REST process
};

#define _ApplyCut(evt) \
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I am not fan of macros. Also, I cannot see where this macro is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

It could used in for example in TRestRawSignalAnalysisProcess, to replace the cut code.

if (ApplyCut()) return nullptr; return fSignalEvent; --> _ApplyCut(fSignalEvent)

I think it is more clear to use this macro.

@nkx111
Copy link
Member Author

nkx111 commented Jun 29, 2022

Hi @nkx, perhaps we could create a dedicated data repository, where we place data files such as the one you are adding v2.2.30_hits.root?

This would be something similar to https://github.com/rest-for-physics/axionlib-data/tree/36629372bcffddd6f95eb12a565c1b15475a64be

That we later make http accessible, and use at the pipelines using wget.

It sounds interesting. We also have several root files used in pipeline for process input, and generator files for NLDBD simulation. Those files are much larger than others.

@nkx111
Copy link
Member Author

nkx111 commented Jun 29, 2022

What the GetParallelDataMembers is doing?

This method is used in another library repository which is not open-sourced yet. It can be useful when you have objects to fill during the process and to summarize after processing. For example we have a process that accumulates value A and B, and calculate C=A/B at the end. Then, in case of multi-thread environment, A, B and C are splited in to several parts, the calculation could be wrong. By using this method, in thread 0, we can retrieve A(B) from thread 1,2,3..., and merge them manually. Then we can correctly calculate C.

@nkx111 nkx111 requested review from lobis and juanangp July 8, 2022 06:37
@nkx111 nkx111 merged commit d1a9e8c into master Jul 12, 2022
@lobis lobis deleted the nkx111-patch-4 branch July 12, 2022 07:45
@jgalan jgalan restored the nkx111-patch-4 branch September 1, 2022 08:13
jgalan added a commit that referenced this pull request Sep 1, 2022
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.

4 participants