Conversation
|
Hi @Etho-b02,
which require these tests: build. @Mu2e/fnalbuild-users, @Mu2e/write have access to CI actions on main. 📝 The author of this pull request is not a member of the Mu2e github organisation. |
AndrewEdmonds11
left a comment
AndrewEdmonds11
left a comment
There was a problem hiding this comment.
Thanks for opening the PR, Bryan. This looks good. Just a few in-line comments we should work through, Let me know if you have any questions. Thanks
| std::unique_ptr<mu2e::STMWaveformDigiCollection> raw_waveform_digis(new mu2e::STMWaveformDigiCollection); | ||
| std::unique_ptr<mu2e::STMWaveformDigiCollection> zs_waveform_digis(new mu2e::STMWaveformDigiCollection); | ||
| std::unique_ptr<mu2e::STMPHDigiCollection> ph_digis(new mu2e::STMPHDigiCollection); | ||
| std::unique_ptr<mu2e::STMWaveformDigiCollection> raw_header_waveform_digis(new mu2e::STMWaveformDigiCollection); |
There was a problem hiding this comment.
I think since this module is just for writing the binary file for debugging, we don't need to create the Offline data products. You can remove these lines and others in this module
| ContainerFragID = frag.fragmentID(); | ||
| if (_verbosityLevel >= 3){std::cout << "\nFrag_id : " << ContainerFragID << "\n";} | ||
|
|
||
| if(ContainerFragID == 103) {isHPGe = true;} else if (ContainerFragID == 203){isLaBr = true ;} |
There was a problem hiding this comment.
I think we don't want to hardcode the numbers 103 and 203. Is it possible to define them in the STMFragment.hh file?
| struct Config | ||
| { | ||
| fhicl::Atom<art::InputTag> stmTag {fhicl::Name("stmTag"), fhicl::Comment("stmTag for new file")}; | ||
| // // fhicl::Atom<int> diagLevel{fhicl::Name("diagLevel"), fhicl::Comment("diagnostic Level")}; |
There was a problem hiding this comment.
You can remove commented out lines like this to clean up the code
| double _peak_fitTime1; // fit time of first rising edge (ns) // TODO: remove this parameter | ||
| double _peak_fitTime2; // fit time of second rising edge (ns) // TODO: remove this parameter | ||
| double _peak_sep; // separation time (ns) // TODO: remove this parameter |
There was a problem hiding this comment.
Can we remove these parameters now, or are they being used somewhere else in the code?
| @@ -35,16 +35,17 @@ namespace mu2e { | |||
| double peak_fitTime2 () const { return _peak_fitTime2; } | |||
| double peak_sep () const { return _peak_sep; } | |||
| const std::vector<int16_t>& adcs () const { return _adcs; } | |||
|
|
|||
| void set_data ( size_t n_data, int16_t const* data ) { _adcs.resize(n_data); std::copy(data, data+n_data, _adcs.begin()); } // TODO: remove this method | |||
There was a problem hiding this comment.
I think we can remove that // TODO comment because I think we want to keep this function
| _stmWaveformDigisToken(consumes<STMWaveformDigiCollection>(config().stmWaveformDigisTag())), | ||
| _subtractPedestal(config().subtractPedestal()), | ||
| _xAxis(config().xAxis()), | ||
| _verbosityLevel(config().verbosityLevel()), | ||
| _channel(STMUtils::getChannel(config().stmWaveformDigisTag())) | ||
| _channel(STMChannel::findByName("HPGe")) // FIXME: don't hardcode this probably don't want to do what we had before and try to infer it from the art::InputTag like this "STMUtils::getChannel(config().stmWaveformDigisTag()))" |
There was a problem hiding this comment.
Let's put this FIXME on our to-do list to fix ASAP. Maybe we can even get it in before this PR completes review...
3 participants
An update version of the offline unpacking module which reads in container fragments. Container fragment represents corresponding detector data. Also includes an event and job summary of what the module reads.