Initial version of gaus_hit_finder example#5
Initial version of gaus_hit_finder example#5wddgit wants to merge 1 commit intoFramework-R-D:mainfrom
Conversation
|
This was from Beojan on SLACK. What level of review are you looking for here? For instance while the file you're printing has a CSV-style header, it isn't a CSV because it has a newline after each field. Is that to match a file produced by the existing GausHitFinder? and I'd suggest using fmt::print instead of iostreams, which can really clean up that code. |
|
And my reply. Thanks. Beojan. I will change it to .txt and use |
wddgit
force-pushed
the
migrateGausHitFinder
branch
from
9f38888 to
b83228e
Compare
|
I force pushed a modification. The most significant part of implements Beojan's suggestions. It changes from using |
|
FYI @beojan Just to make sure you are receiving comments on this PR. |
| std::ofstream out(filename, std::ios::binary); | ||
| if (!out) { | ||
| throw std::runtime_error("Failed to open file for writing: " + filename); | ||
| return false; |
There was a problem hiding this comment.
This return false; will not execute. Suggest removing
| std::ifstream in(filename, std::ios::binary); | ||
| if (!in) { | ||
| throw std::runtime_error("Failed to open file for reading: " + filename); | ||
| return false; |
There was a problem hiding this comment.
This return false; will not execute. Suggest removing
|
|
||
| void print_hits_to_file(std::vector<recob::Hit> const& hits, | ||
| std::string const& filename) { | ||
| std::FILE* file = std::fopen(filename.c_str(), "w"); |
There was a problem hiding this comment.
| std::FILE* file = std::fopen(filename.c_str(), "w"); | |
| std::ofstream file{filename}; |
| std::FILE* file = std::fopen(filename.c_str(), "w"); | ||
| if (!file) { | ||
| throw std::runtime_error("Failed to open file for writing: " + filename); | ||
| return; |
There was a problem hiding this comment.
Suggest removing this return; statement as it will not be executed.
| // hit.SignalType() | ||
| // hit.WireID() | ||
| } | ||
| std::fclose(file); |
There was a problem hiding this comment.
The explicit std::fclose(...) can be removed if using the std::ofstream approach above.
| bool read_wires_from_file(std::string const& filename, | ||
| std::vector<recob::Wire>& wires) { |
There was a problem hiding this comment.
Instead of passing in an output argument, I suggest changing the interface:
std::optional<std::vector<recob::Wire>> read_wires_from_file(std::string const& filename)
{
std::vector<recob::Wire> wires;
...
return wires;
}and then early returns (like return false;) are replaced with return std::nullopt;
| in.close(); | ||
| return in.good(); |
There was a problem hiding this comment.
See above suggestions, such that:
| in.close(); | |
| return in.good(); | |
| return wires; |
| std::vector<recob::Wire> wires; | ||
| bool status = read_wires_from_file(filename, wires); | ||
| if (!status) { | ||
| throw std::runtime_error("Failure while reading from file: " + filename); | ||
| } | ||
| return wires; |
There was a problem hiding this comment.
Given the above changes, this would change to:
| std::vector<recob::Wire> wires; | |
| bool status = read_wires_from_file(filename, wires); | |
| if (!status) { | |
| throw std::runtime_error("Failure while reading from file: " + filename); | |
| } | |
| return wires; | |
| if (auto wires = read_wires_from_file(filename)) { | |
| return *wires; | |
| } | |
| throw std::runtime_error("Failure while reading from file: " + filename); |
2 participants
This is an initial version of the result of the migration of the module defined in GausHitFinder_module.cc in the larreco repository of LArSoft from the existing version designed to run under the
artframework to a version that will run under thephlexframework. The purpose of this code is to serve as an example for future migrations and also to be used in prototype testing. It is not intended to be used for reconstruction of data for use in physics analysis. That code should be placed in some other repository and maintained by other groups.See the README.md (best viewed in GitHub) for more information.