From 88e4ee71aa24746f4b048ecc09e18875151e8ec2 Mon Sep 17 00:00:00 2001 From: Kyle Knoepfel Date: Fri, 10 Jul 2026 15:28:32 -0500 Subject: [PATCH] Enable pre-commit and apply chnages --- .clang-format | 180 +++++------ .gersemirc | 13 + .markdownlint.yaml | 2 + .pre-commit-config.yaml | 60 ++++ README.md | 8 +- migration/doc/conf.py | 39 +-- migration/doc/index.rst | 1 - migration/gauss_hit_finder/CMakeLists.txt | 160 ++++----- migration/gauss_hit_finder/README.md | 306 +++++++++--------- migration/gauss_hit_finder/compare_hits.py | 7 +- .../.clang-format | 1 + .../find_hits_with_gaussians.cpp | 140 ++++---- .../find_hits_with_gaussians.hpp | 27 +- .../find_hits_with_gaussians_design1.cpp | 12 +- .../find_hits_with_gaussians_design1.hpp | 27 +- .../gauss_hit_finder/print_hits_to_file.cpp | 9 +- .../gauss_hit_finder/print_hits_to_file.hpp | 9 +- .../register_find_hits_with_gaussians.cpp | 55 ++-- ...ister_find_hits_with_gaussians_cell_id.cpp | 5 +- ...ister_find_hits_with_gaussians_design1.cpp | 64 ++-- .../register_print_hits_to_file.cpp | 5 +- .../test_find_hits_with_gaussians.jsonnet | 2 +- ...t_find_hits_with_gaussians_design1.jsonnet | 2 +- .../gauss_hit_finder/wire_serialization.cpp | 247 +++++++------- .../gauss_hit_finder/wire_serialization.hpp | 11 +- migration/gauss_hit_finder/wires_source.cpp | 20 +- scripts/fix_header_guards.py | 147 +++++++++ split_square_verify.cpp | 20 +- subtract.py | 7 +- 29 files changed, 909 insertions(+), 677 deletions(-) create mode 100644 .gersemirc create mode 100644 .markdownlint.yaml create mode 100644 .pre-commit-config.yaml create mode 100644 migration/gauss_hit_finder/copied_from_larsoft_minor_edits/.clang-format create mode 100755 scripts/fix_header_guards.py diff --git a/.clang-format b/.clang-format index d94a292..a5890c8 100644 --- a/.clang-format +++ b/.clang-format @@ -1,75 +1,75 @@ --- -Language: Cpp +Language: Cpp AccessModifierOffset: -2 AlignAfterOpenBracket: Align AlignArrayOfStructures: None AlignConsecutiveAssignments: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: true + PadOperators: true AlignConsecutiveBitFields: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignConsecutiveDeclarations: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: true AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignConsecutiveMacros: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignConsecutiveShortCaseStatements: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false + AcrossComments: false AlignCaseArrows: false AlignCaseColons: false AlignConsecutiveTableGenBreakingDAGArgColons: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignConsecutiveTableGenCondOperatorColons: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignConsecutiveTableGenDefinitionColons: - Enabled: false + Enabled: false AcrossEmptyLines: false - AcrossComments: false - AlignCompound: false + AcrossComments: false + AlignCompound: false AlignFunctionDeclarations: false AlignFunctionPointers: false - PadOperators: false + PadOperators: false AlignEscapedNewlines: Right -AlignOperands: Align +AlignOperands: Align AlignTrailingComments: - Kind: Always - OverEmptyLines: 0 + Kind: Always + OverEmptyLines: 0 AllowAllArgumentsOnNextLine: true AllowAllParametersOfDeclarationOnNextLine: true AllowBreakBeforeNoexceptSpecifier: Never @@ -89,21 +89,21 @@ BinPackArguments: false BinPackParameters: false BitFieldColonSpacing: Both BraceWrapping: - AfterCaseLabel: false - AfterClass: false + AfterCaseLabel: false + AfterClass: false AfterControlStatement: Never - AfterEnum: false + AfterEnum: false AfterExternBlock: false - AfterFunction: true - AfterNamespace: false + AfterFunction: true + AfterNamespace: false AfterObjCDeclaration: false - AfterStruct: false - AfterUnion: false - BeforeCatch: false - BeforeElse: false + AfterStruct: false + AfterUnion: false + BeforeCatch: false + BeforeElse: false BeforeLambdaBody: false - BeforeWhile: false - IndentBraces: false + BeforeWhile: false + IndentBraces: false SplitEmptyFunction: true SplitEmptyRecord: false SplitEmptyNamespace: false @@ -111,7 +111,7 @@ BreakAdjacentStringLiterals: true BreakAfterAttributes: Leave BreakAfterJavaFieldAnnotations: false BreakAfterReturnType: None -BreakArrays: true +BreakArrays: true BreakBeforeBinaryOperators: None BreakBeforeConceptDeclarations: Always BreakBeforeBraces: Custom @@ -123,14 +123,14 @@ BreakFunctionDefinitionParameters: false BreakInheritanceList: AfterColon BreakStringLiterals: true BreakTemplateDeclarations: Yes -ColumnLimit: 100 -CommentPragmas: '^ IWYU pragma:' +ColumnLimit: 100 +CommentPragmas: "^ IWYU pragma:" CompactNamespaces: false ConstructorInitializerIndentWidth: 2 ContinuationIndentWidth: 2 Cpp11BracedListStyle: true DerivePointerAlignment: false -DisableFormat: false +DisableFormat: false EmptyLineAfterAccessModifier: Never EmptyLineBeforeAccessModifier: LogicalBlock ExperimentalAutoDetectBinPacking: false @@ -141,22 +141,22 @@ ForEachMacros: - BOOST_FOREACH IfMacros: - KJ_IF_MAYBE -IncludeBlocks: Preserve +IncludeBlocks: Preserve IncludeCategories: - - Regex: '^"(llvm|llvm-c|clang|clang-c)/' - Priority: 2 - SortPriority: 0 - CaseSensitive: false - - Regex: '^(<|"(gtest|gmock|isl|json)/)' - Priority: 3 - SortPriority: 0 - CaseSensitive: false - - Regex: '.*' - Priority: 1 - SortPriority: 0 - CaseSensitive: false -IncludeIsMainRegex: '(Test)?$' -IncludeIsMainSourceRegex: '' + - Regex: '^"(llvm|llvm-c|clang|clang-c)/' + Priority: 2 + SortPriority: 0 + CaseSensitive: false + - Regex: '^(<|"(gtest|gmock|isl|json)/)' + Priority: 3 + SortPriority: 0 + CaseSensitive: false + - Regex: ".*" + Priority: 1 + SortPriority: 0 + CaseSensitive: false +IncludeIsMainRegex: "(Test)?$" +IncludeIsMainSourceRegex: "" IndentAccessModifiers: false IndentCaseBlocks: false IndentCaseLabels: false @@ -165,29 +165,29 @@ IndentExternBlock: AfterExternBlock IndentGotoLabels: true IndentPPDirectives: None IndentRequiresClause: true -IndentWidth: 2 +IndentWidth: 2 IndentWrappedFunctionNames: false -InsertBraces: false +InsertBraces: false InsertNewlineAtEOF: false InsertTrailingCommas: None IntegerLiteralSeparator: - Binary: 0 + Binary: 0 BinaryMinDigits: 0 - Decimal: 0 + Decimal: 0 DecimalMinDigits: 0 - Hex: 0 - HexMinDigits: 0 + Hex: 0 + HexMinDigits: 0 JavaScriptQuotes: Leave JavaScriptWrapImports: true KeepEmptyLines: - AtEndOfFile: false - AtStartOfBlock: true - AtStartOfFile: false -KeepFormFeed: false + AtEndOfFile: false + AtStartOfBlock: true + AtStartOfFile: false +KeepFormFeed: false LambdaBodyIndentation: Signature -LineEnding: DeriveLF -MacroBlockBegin: '' -MacroBlockEnd: '' +LineEnding: DeriveLF +MacroBlockBegin: "" +MacroBlockEnd: "" MainIncludeChar: Quote MaxEmptyLinesToKeep: 1 NamespaceIndentation: All @@ -210,10 +210,10 @@ PenaltyExcessCharacter: 1000000 PenaltyIndentedWhitespace: 0 PenaltyReturnTypeOnItsOwnLine: 200 PointerAlignment: Left -PPIndentWidth: -1 +PPIndentWidth: -1 QualifierAlignment: Right ReferenceAlignment: Pointer -ReflowComments: false +ReflowComments: false RemoveBracesLLVM: false RemoveEmptyLinesInUnwrappedLines: false RemoveParentheses: Leave @@ -223,7 +223,7 @@ RequiresExpressionIndentation: OuterScope SeparateDefinitionBlocks: Leave ShortNamespaceLines: 1 SkipMacroDefinitionBody: false -SortIncludes: CaseSensitive +SortIncludes: CaseSensitive SortJavaStaticImport: Before SortUsingDeclarations: LexicographicNumeric SpaceAfterCStyleCast: false @@ -242,7 +242,7 @@ SpaceBeforeParensOptions: AfterForeachMacros: true AfterFunctionDefinitionName: false AfterFunctionDeclarationName: false - AfterIfMacros: true + AfterIfMacros: true AfterOverloadedOperator: false AfterPlacementOperator: true AfterRequiresInClause: false @@ -252,28 +252,28 @@ SpaceBeforeRangeBasedForLoopColon: true SpaceBeforeSquareBrackets: false SpaceInEmptyBlock: false SpacesBeforeTrailingComments: 1 -SpacesInAngles: Never +SpacesInAngles: Never SpacesInContainerLiterals: true SpacesInLineCommentPrefix: - Minimum: 1 - Maximum: -1 -SpacesInParens: Never + Minimum: 1 + Maximum: -1 +SpacesInParens: Never SpacesInParensOptions: ExceptDoubleParentheses: false - InCStyleCasts: false + InCStyleCasts: false InConditionalStatements: false InEmptyParentheses: false - Other: false + Other: false SpacesInSquareBrackets: false -Standard: Latest +Standard: Latest StatementAttributeLikeMacros: - Q_EMIT StatementMacros: - Q_UNUSED - QT_REQUIRE_VERSION TableGenBreakInsideDAGArg: DontBreak -TabWidth: 8 -UseTab: Never +TabWidth: 8 +UseTab: Never VerilogBreakBetweenInstancePorts: true WhitespaceSensitiveMacros: - BOOST_PP_STRINGIZE diff --git a/.gersemirc b/.gersemirc new file mode 100644 index 0000000..acc04e9 --- /dev/null +++ b/.gersemirc @@ -0,0 +1,13 @@ +# gersemi is an opinionated formatter and does not support the same +# level of configuration as cmake-format. The previous settings from +# .cmake-format.json (dangle_align and dangle_parens) are not +# supported and have no direct equivalents. + +definitions: [] +disable_formatting: false +extensions: [] +indent: 2 +line_length: 99 +list_expansion: favour-inlining +unsafe: false +warn_about_unknown_commands: false diff --git a/.markdownlint.yaml b/.markdownlint.yaml new file mode 100644 index 0000000..e19cae0 --- /dev/null +++ b/.markdownlint.yaml @@ -0,0 +1,2 @@ +MD013: + code_blocks: false diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..85b9a54 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,60 @@ +# See https://pre-commit.com for more information +# See https://pre-commit.com/hooks.html for more hooks + +# Everything in copied_from_larsoft_minor_edits is copied from larsoft and should not be modified. +# It is excluded from pre-commit checks to avoid unnecessary noise. +exclude: ^migration/gauss_hit_finder/copied_from_larsoft_minor_edits/ + +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v6.0.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-toml + - id: check-merge-conflict + - id: check-vcs-permalinks + - id: check-executables-have-shebangs + - id: mixed-line-ending + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.15.19 + hooks: + - id: ruff-format + types_or: [python, pyi] + - id: ruff + types_or: [python, pyi] + - repo: https://github.com/pre-commit/mirrors-clang-format + rev: v20.1.8 + hooks: + - id: clang-format + types_or: [c++, c, cuda] + - repo: local + hooks: + - id: fix-header-guards + name: fix C++ header guards + entry: python3 scripts/fix_header_guards.py + language: system + files: \.(h|hpp)$ + - repo: https://github.com/BlankSpruce/gersemi-pre-commit + rev: 0.27.7 + hooks: + - id: gersemi + - repo: https://github.com/google/go-jsonnet + rev: v0.22.0 + hooks: + - id: jsonnet-format + - id: jsonnet-lint + # The files in test/max-parallelism import files that are generated during the build. + # We therefore exclude them from linting, but still format them. + exclude: ^test/max-parallelism + - repo: https://github.com/rbubley/mirrors-prettier + rev: v3.8.4 + hooks: + - id: prettier + types_or: [yaml] + - repo: https://github.com/DavidAnson/markdownlint-cli2 + rev: v0.22.1 + hooks: + - id: markdownlint-cli2 + args: [--fix] diff --git a/README.md b/README.md index 1eae253..75cc85d 100644 --- a/README.md +++ b/README.md @@ -11,12 +11,14 @@ git clone -b phlex-v0.3 https://github.com/Framework-R-D/phlex-examples.git ``` Now create a build directory: + ```console mkdir build-phlex-examples cd build-phlex-examples ``` -The repository can now be easily built by activating [the environment you created when installing Phlex](https://github.com/Framework-R-D/phlex/blob/main/INSTALLATION.md#creating-and-installing-the-phlex-environment): +The repository can now be easily built by activating +[the environment you created when installing Phlex](https://github.com/Framework-R-D/phlex/blob/main/INSTALLATION.md#creating-and-installing-the-phlex-environment): ```console spack env activate my-phlex-environment @@ -94,7 +96,8 @@ Processed layers: ### Job with Python algorithms -To run a job that uses a Python algorithm, the `PYTHONPATH` environment variable must be adjusted to include the directory with the Python module: +To run a job that uses a Python algorithm, the `PYTHONPATH` environment +variable must be adjusted to include the directory with the Python module: ```console export PYTHONPATH=$(realpath ../phlex-examples):${PYTHONPATH} @@ -117,6 +120,7 @@ Processed layers: > [!NOTE] > For Phlex v0.1.0, Python data products: +> > - Cannot be written to output files > - Must belong to the `"job"` data layer > diff --git a/migration/doc/conf.py b/migration/doc/conf.py index 26244dc..a843901 100644 --- a/migration/doc/conf.py +++ b/migration/doc/conf.py @@ -7,16 +7,20 @@ import subprocess import sys -sys.path.insert(0, os.path.abspath('.') + '/_extensions') +sys.path.insert(0, os.path.abspath(".") + "/_extensions") # -- Project information ----------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#project-information -project = 'Phlex Migration Guide' -copyright = '2026, Fermi Forward Discovery Group, LLC' -author = '' +project = "Phlex Migration Guide" +copyright = "2026, Fermi Forward Discovery Group, LLC" +author = "" -git_commit = subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD']).decode('ascii').strip() +git_commit = ( + subprocess.check_output(["git", "rev-parse", "--short", "HEAD"]) + .decode("ascii") + .strip() +) release = f"0.1.alpha ({git_commit})" @@ -24,16 +28,16 @@ # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration extensions = [ - 'sphinx.ext.autosectionlabel', + "sphinx.ext.autosectionlabel", ] autosectionlabel_prefix_document = True numfig = True numfig_format = { - 'code-block': 'Listing %s', - 'figure': 'Fig. %s', - 'section': 'Section %s', - 'table': 'Table %s', + "code-block": "Listing %s", + "figure": "Fig. %s", + "section": "Section %s", + "table": "Table %s", } @@ -52,9 +56,9 @@ # -- Options for HTML output ------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output -html_theme = 'bizstyle' -html_static_path = ['_static'] -html_style = 'phlexframework.css' +html_theme = "bizstyle" +html_static_path = ["_static"] +html_style = "phlexframework.css" html_last_updated_fmt = "" # Equivalent to showing (e.g.) May 29, 2025 # -- Options for LaTeX output ------------------------------------------------ @@ -62,9 +66,6 @@ latex_elements = {} -latex_documents = [("index", - "phlex-migration-guide.tex", - project, - author, - "manual", - False)] +latex_documents = [ + ("index", "phlex-migration-guide.tex", project, author, "manual", False) +] diff --git a/migration/doc/index.rst b/migration/doc/index.rst index 3c54973..79d4d43 100644 --- a/migration/doc/index.rst +++ b/migration/doc/index.rst @@ -13,4 +13,3 @@ algorithms. :numbered: introduction - \ No newline at end of file diff --git a/migration/gauss_hit_finder/CMakeLists.txt b/migration/gauss_hit_finder/CMakeLists.txt index db41623..6359926 100644 --- a/migration/gauss_hit_finder/CMakeLists.txt +++ b/migration/gauss_hit_finder/CMakeLists.txt @@ -1,17 +1,19 @@ # Proxies for framework-agnostic experiment libraries # N.B. The specified library type should be SHARED. -add_library(find_hits_with_gaussians SHARED - find_hits_with_gaussians.cpp - find_hits_with_gaussians_design1.cpp - copied_from_larsoft_minor_edits/Hit.cxx - copied_from_larsoft_minor_edits/geo_types.cxx - copied_from_larsoft_minor_edits/CandHitStandard.cxx - copied_from_larsoft_minor_edits/PeakFitterMrqdt.cxx - copied_from_larsoft_minor_edits/HitFilterAlg.cxx - copied_from_larsoft_minor_edits/MarqFitAlg.cxx - copied_from_larsoft_minor_edits/Wire.cxx - wire_serialization.cpp - print_hits_to_file.cpp +add_library( + find_hits_with_gaussians + SHARED + find_hits_with_gaussians.cpp + find_hits_with_gaussians_design1.cpp + copied_from_larsoft_minor_edits/Hit.cxx + copied_from_larsoft_minor_edits/geo_types.cxx + copied_from_larsoft_minor_edits/CandHitStandard.cxx + copied_from_larsoft_minor_edits/PeakFitterMrqdt.cxx + copied_from_larsoft_minor_edits/HitFilterAlg.cxx + copied_from_larsoft_minor_edits/MarqFitAlg.cxx + copied_from_larsoft_minor_edits/Wire.cxx + wire_serialization.cpp + print_hits_to_file.cpp ) target_link_libraries(find_hits_with_gaussians PRIVATE TBB::tbb fmt::fmt) @@ -20,25 +22,28 @@ add_library(wires_source MODULE wires_source.cpp) target_link_libraries(wires_source PRIVATE phlex::module find_hits_with_gaussians) # Register experiment libraries with Phlex -add_library(find_hits_with_gaussians_hof MODULE - register_find_hits_with_gaussians.cpp) -target_link_libraries(find_hits_with_gaussians_hof PRIVATE - phlex::module find_hits_with_gaussians) +add_library(find_hits_with_gaussians_hof MODULE register_find_hits_with_gaussians.cpp) +target_link_libraries(find_hits_with_gaussians_hof PRIVATE phlex::module find_hits_with_gaussians) -add_library(find_hits_with_gaussians_cell_id_hof MODULE - register_find_hits_with_gaussians_cell_id.cpp) -target_link_libraries(find_hits_with_gaussians_cell_id_hof PRIVATE - phlex::module) +add_library( + find_hits_with_gaussians_cell_id_hof + MODULE + register_find_hits_with_gaussians_cell_id.cpp +) +target_link_libraries(find_hits_with_gaussians_cell_id_hof PRIVATE phlex::module) -add_library(print_hits_to_file_hof MODULE - register_print_hits_to_file.cpp) -target_link_libraries(print_hits_to_file_hof PRIVATE - phlex::module find_hits_with_gaussians) +add_library(print_hits_to_file_hof MODULE register_print_hits_to_file.cpp) +target_link_libraries(print_hits_to_file_hof PRIVATE phlex::module find_hits_with_gaussians) -add_library(find_hits_with_gaussians_design1_hof MODULE - register_find_hits_with_gaussians_design1.cpp) -target_link_libraries(find_hits_with_gaussians_design1_hof PRIVATE - phlex::module find_hits_with_gaussians) +add_library( + find_hits_with_gaussians_design1_hof + MODULE + register_find_hits_with_gaussians_design1.cpp +) +target_link_libraries( + find_hits_with_gaussians_design1_hof + PRIVATE phlex::module find_hits_with_gaussians +) file(GLOB test_files ${CMAKE_CURRENT_SOURCE_DIR}/*.dat) file(COPY ${test_files} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) @@ -46,9 +51,11 @@ file(COPY ${test_files} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) file(GLOB test_files_art ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_*.txt) file(COPY ${test_files_art} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) -add_test(NAME GausHitFinderTest - COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/test_find_hits_with_gaussians.jsonnet - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +add_test( + NAME GausHitFinderTest + COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/test_find_hits_with_gaussians.jsonnet + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} +) # iterates 0 to 4 inclusive, creating a test to compare the output hits of # the phlex job to the output hits of the art job for each event (5 events). @@ -60,48 +67,50 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux") foreach(n RANGE 4) add_test( NAME GausHitFinderCompare_${n} - COMMAND diff -q -s - ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt - ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt + COMMAND + diff -q -s ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt + ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt ) set_tests_properties(GausHitFinderCompare_${n} PROPERTIES DEPENDS GausHitFinderTest) endforeach() -# On a Mac, we expect the output to be slightly -# different because the reference files were -# generated on a Linux machine. + # On a Mac, we expect the output to be slightly + # different because the reference files were + # generated on a Linux machine. elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") find_package(Python3 REQUIRED COMPONENTS Interpreter) foreach(n RANGE 4) add_test( NAME GausHitFinderCompare_${n} - COMMAND ${Python3_EXECUTABLE} - ${CMAKE_CURRENT_SOURCE_DIR}/compare_hits.py - ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt - ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt + COMMAND + ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/compare_hits.py + ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt ) set_tests_properties(GausHitFinderCompare_${n} PROPERTIES DEPENDS GausHitFinderTest) endforeach() -# Always fail on other platforms -# This will warn us to add a new comparison -# method if we ever run on a new platform. + # Always fail on other platforms + # This will warn us to add a new comparison + # method if we ever run on a new platform. else() foreach(n RANGE 4) - add_test( - NAME GausHitFinderCompare_${n} - COMMAND ${CMAKE_COMMAND} -E false - ) + add_test(NAME GausHitFinderCompare_${n} COMMAND ${CMAKE_COMMAND} -E false) set_tests_properties(GausHitFinderCompare_${n} PROPERTIES DEPENDS GausHitFinderTest) endforeach() endif() # Repeat for the version with unfold-transform-fold instead of a single transform. -add_test(NAME GausHitFinderTestWithUnfold - COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/test_find_hits_with_gaussians_design1.jsonnet - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +add_test( + NAME GausHitFinderTestWithUnfold + COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/test_find_hits_with_gaussians_design1.jsonnet + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} +) # The tests must not be run concurrently because they read and write to the same file. -set_tests_properties(GausHitFinderTestWithUnfold PROPERTIES DEPENDS - "GausHitFinderCompare_0;GausHitFinderCompare_1;GausHitFinderCompare_2;GausHitFinderCompare_3;GausHitFinderCompare_4") +set_tests_properties( + GausHitFinderTestWithUnfold + PROPERTIES + DEPENDS + "GausHitFinderCompare_0;GausHitFinderCompare_1;GausHitFinderCompare_2;GausHitFinderCompare_3;GausHitFinderCompare_4" +) # The reference files were generated on a Linux machine # so we expect identical output on Linux @@ -109,36 +118,41 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux") foreach(n RANGE 4) add_test( NAME GausHitFinderCompareWithUnfold_${n} - COMMAND diff -q -s - ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt - ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt - ) - set_tests_properties(GausHitFinderCompareWithUnfold_${n} PROPERTIES DEPENDS GausHitFinderTestWithUnfold) + COMMAND + diff -q -s ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt + ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt + ) + set_tests_properties( + GausHitFinderCompareWithUnfold_${n} + PROPERTIES DEPENDS GausHitFinderTestWithUnfold + ) endforeach() -# On a Mac, we expect the output to be slightly -# different because the reference files were -# generated on a Linux machine. + # On a Mac, we expect the output to be slightly + # different because the reference files were + # generated on a Linux machine. elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") foreach(n RANGE 4) add_test( NAME GausHitFinderCompareWithUnfold_${n} - COMMAND ${Python3_EXECUTABLE} - ${CMAKE_CURRENT_SOURCE_DIR}/compare_hits.py - ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt - ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt + COMMAND + ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/compare_hits.py + ${CMAKE_CURRENT_SOURCE_DIR}/art_hits_${n}.txt ${CMAKE_CURRENT_BINARY_DIR}/hits_${n}.txt + ) + set_tests_properties( + GausHitFinderCompareWithUnfold_${n} + PROPERTIES DEPENDS GausHitFinderTestWithUnfold ) - set_tests_properties(GausHitFinderCompareWithUnfold_${n} PROPERTIES DEPENDS GausHitFinderTestWithUnfold) endforeach() -# Always fail on other platforms -# This will warn us to add a new comparison -# method if we ever run on a new platform. + # Always fail on other platforms + # This will warn us to add a new comparison + # method if we ever run on a new platform. else() foreach(n RANGE 4) - add_test( - NAME GausHitFinderCompareWithUnfold_${n} - COMMAND ${CMAKE_COMMAND} -E false + add_test(NAME GausHitFinderCompareWithUnfold_${n} COMMAND ${CMAKE_COMMAND} -E false) + set_tests_properties( + GausHitFinderCompareWithUnfold_${n} + PROPERTIES DEPENDS GausHitFinderTestWithUnfold ) - set_tests_properties(GausHitFinderCompareWithUnfold_${n} PROPERTIES DEPENDS GausHitFinderTestWithUnfold) endforeach() endif() diff --git a/migration/gauss_hit_finder/README.md b/migration/gauss_hit_finder/README.md index c9ecdb6..31ee2f6 100644 --- a/migration/gauss_hit_finder/README.md +++ b/migration/gauss_hit_finder/README.md @@ -1,60 +1,51 @@ # Purpose of this directory -This directory holds an example showing how to convert a -module that works with the `art` framework into -algorithms that work with the `phlex` framework. This -particular example converts the module defined in -the file GausHitFinder_module.cc from the LArSoft larreco -repository. - -This is only an example and a prototype to be used in -testing. It is not intended to be the version of this -module used by DUNE or any other experiment -for production or physics. That belongs in some other -repository. - -This example is based on `main` branch of `phlex` as of the -last time the example was updated. It might need modifications -to work with the newest version of `phlex`. The migration to -phlex started with version of GausHitFinder_module.cc in -v10_05_00 of larreco which was the version in use with DUNE -software at the time the migration work was started. If there -is a request, I can update the example to a more recent version. - -# Work in Progress - -This is work in progress. It is not final. It is intended -that this example will change as we learn more about using -`phlex` and as we receive feedback from DUNE. Currently, this -is more like a rough first draft of our first attempts -to use `phlex`. Please don't expect stability. Many features -of `phlex` have not been implemented and the design of `phlex` -itself is changing. Those changes will also cause this example -to change. - -As we study this example, it will guide how phlex is used. -As we run tests, it may point to problems in phlex and this -may guide which areas developers should focus on to fix -issues and remove performance bottlenecks. - -# Files in this directory +This directory holds an example showing how to convert a module that works +with the `art` framework into algorithms that work with the `phlex` framework. +This particular example converts the module defined in the file +GausHitFinder_module.cc from the LArSoft larreco repository. + +This is only an example and a prototype to be used in testing. It is not +intended to be the version of this module used by DUNE or any other experiment +for production or physics. That belongs in some other repository. + +This example is based on `main` branch of `phlex` as of the last time the +example was updated. It might need modifications to work with the newest +version of `phlex`. The migration to phlex started with version of +GausHitFinder_module.cc in v10_05_00 of larreco which was the version in use +with DUNE software at the time the migration work was started. If there is a +request, I can update the example to a more recent version. + +## Work in Progress + +This is work in progress. It is not final. It is intended that this example +will change as we learn more about using `phlex` and as we receive feedback +from DUNE. Currently, this is more like a rough first draft of our first +attempts to use `phlex`. Please don't expect stability. Many features of +`phlex` have not been implemented and the design of `phlex` itself is changing. +Those changes will also cause this example to change. + +As we study this example, it will guide how phlex is used. As we run tests, +it may point to problems in phlex and this may guide which areas developers +should focus on to fix issues and remove performance bottlenecks. + +## Files in this directory ## Interesting files that are the heart of the example -There are multiple prototypes to explore different -approaches to migrating GausHitFinder to `phlex`. +There are multiple prototypes to explore different approaches to migrating +GausHitFinder to `phlex`. -These files are an example where one large `phlex` -algorithm replaces GausHitFinder. +These files are an example where one large `phlex` algorithm replaces +GausHitFinder. 1. find_hits_with_gaussians.hpp 2. find_hits_with_gaussians.cpp 3. register_find_hits_with_gaussians.cpp 4. test_find_hits_with_gaussians.jsonnet -These files are an example where there is an -unfold algorithm, followed by a transform algorithm, -and finally a fold algorithm. The outer parallel_for +These files are an example where there is an unfold algorithm, followed by a +transform algorithm, and finally a fold algorithm. The outer parallel_for iteration is replaced by that sequence of algorithms. 1. find_hits_with_gaussians_design1.hpp @@ -62,21 +53,18 @@ iteration is replaced by that sequence of algorithms. 3. register_find_hits_with_gaussians_design1.cpp 4. test_find_hits_with_gaussians_design1.jsonnet -We plan to implement more prototype migrations -of GausHitFinder in the future to explore the -possibilities and execute tests. +We plan to implement more prototype migrations of GausHitFinder in the future +to explore the possibilities and execute tests. ## These files exist only for test purposes -There is one algorithm that will print out reconstructed hits. -This printout can be used to compare results between an -`art` process and a `phlex` process running GausHitFinder. -There are functions that can be used to persistently store -the input from an `art` process (a vector of `Wire`) so -it can be used in the `phlex` process. And there is a `phlex` provider -to read this input. This persistence mechanism is necessary -because there is not an alternative yet. These files are all -temporary and not part of the migration example. +There is one algorithm that will print out reconstructed hits. This printout +can be used to compare results between an `art` process and a `phlex` process +running GausHitFinder. There are functions that can be used to persistently +store the input from an `art` process (a vector of `Wire`) so it can be used +in the `phlex` process. And there is a `phlex` provider to read this input. +This persistence mechanism is necessary because there is not an alternative +yet. These files are all temporary and not part of the migration example. 1. print_hits_to_file.hpp 2. print_hits_to_file.cpp @@ -91,75 +79,64 @@ temporary and not part of the migration example. ## Files copied from LArSoft -The infrastructure does not yet exist to link the -code here with LArSoft code or include headers. -Necessary files are copied in with minimal changes. -Some changes are necessary because the files are in -a different location and some remove unwanted dependences, -but the modifications are small. When it is possible -to depend on LArSoft code in some other repository, -these files should be deleted to remove duplication. +The infrastructure does not yet exist to link the code here with LArSoft code +or include headers. Necessary files are copied in with minimal changes. Some +changes are necessary because the files are in a different location and some +remove unwanted dependences, but the modifications are small. When it is +possible to depend on LArSoft code in some other repository, these files +should be deleted to remove duplication. These files are in the subdirectory: ```phlex-examples/migration/gauss_hit_finder/copied_from_larsoft_minor_edits``` -# Where more work is needed - -The first example was implemented as a single `phlex` -transform. Does it make sense to split it with unfolds -and folds? Does it make sense to have multiple transforms? -We don't know the answers to these questions yet. -Getting the single transform to work was a good first step. - -The next thing we implemented was replacing the -outer `parallel_for` with an `unfold`, `transform`, and `fold` -sequence of algorithms. In the future, we plan to create other -versions pushing the division into separate algorithms to lower -levels. Then we plan to run tests and compare the different -versions. - -`Phlex` will not support the `Tools` feature that existed in -`art`. One possibility is that algorithm nodes scheduled -by `tbb::flow_graph` offer sufficient configurability that we -don't need a separate plugin system like `Tools`. -Another possibility is that we eventually implement a -plugin system for `phlex`. For now, I replaced the two `Tool` -types used in `GausHitFinder` with direct instantiations of one -of the plugin types. I instantiated a vector of objects of type -`CandHitStandard` and one object of type `PeakFitterMrqdt`. They -needed some modification but are as close as reasonably possible -to the original versions. - -I do not know yet how to deal with the fact that `GausHitFinder` -can be configured to produce 1 or 2 output data products. -For now, the example can only produce one `std::vector`. -It's configurable whether to filter hits or not, but it cannot -produce both outputs at the same time. - -For now, this ignores the fact that `GausHitFinder` also can -produce `art::Assns` and -`art::Assns`. In the `art` version, -it is configurable whether to produce those or not. -We are ignoring this because `phlex` does not yet support `art::Assns` -or whatever will be used to replace that type. For now, the -example can only produce one `std::vector`. - -Here is a list of the files that were used as a basis -for this example, but significantly modified from the -LArSoft version: - -``` +## Where more work is needed + +The first example was implemented as a single `phlex` transform. Does it make +sense to split it with unfolds and folds? Does it make sense to have multiple +transforms? We don't know the answers to these questions yet. Getting the +single transform to work was a good first step. + +The next thing we implemented was replacing the outer `parallel_for` with an +`unfold`, `transform`, and `fold` sequence of algorithms. In the future, we +plan to create other versions pushing the division into separate algorithms to +lower levels. Then we plan to run tests and compare the different versions. + +`Phlex` will not support the `Tools` feature that existed in `art`. One +possibility is that algorithm nodes scheduled by `tbb::flow_graph` offer +sufficient configurability that we don't need a separate plugin system like +`Tools`. Another possibility is that we eventually implement a plugin system +for `phlex`. For now, I replaced the two `Tool` types used in `GausHitFinder` +with direct instantiations of one of the plugin types. I instantiated a vector +of objects of type `CandHitStandard` and one object of type `PeakFitterMrqdt`. +They needed some modification but are as close as reasonably possible to the +original versions. + +I do not know yet how to deal with the fact that `GausHitFinder` can be +configured to produce 1 or 2 output data products. For now, the example can +only produce one `std::vector`. It's configurable whether to +filter hits or not, but it cannot produce both outputs at the same time. + +For now, this ignores the fact that `GausHitFinder` also can produce +`art::Assns` and +`art::Assns`. In the `art` version, it is +configurable whether to produce those or not. We are ignoring this because +`phlex` does not yet support `art::Assns` or whatever will be used to replace +that type. For now, the example can only produce one `std::vector`. + +Here is a list of the files that were used as a basis for this example, but +significantly modified from the LArSoft version: + +```text https://github.com/LArSoft/larreco/blob/develop/larreco/HitFinder/GausHitFinder_module.cc ``` -Some files were copied with minimal modifications. The "tool" -classes were split into .h and .cxx files and are no longer implemented -as `art Tools`, but otherwise the changes are minimal. -Here is a list of the files that were copied from LArSoft -with minimal modifications: +Some files were copied with minimal modifications. The "tool" classes were +split into .h and .cxx files and are no longer implemented as `art Tools`, but +otherwise the changes are minimal. Here is a list of the files that were +copied from LArSoft with minimal modifications: -``` +```text https://github.com/LArSoft/larreco/blob/develop/larreco/HitFinder/HitFinderTools/CandHitStandard_tool.cc https://github.com/LArSoft/larreco/blob/develop/larreco/HitFinder/HitFinderTools/PeakFitterMrqdt_tool.cc https://github.com/LArSoft/larreco/blob/develop/larreco/HitFinder/HitFilterAlg.h @@ -180,53 +157,64 @@ with minimal modifications: ``` For now, the part of `GausHitFinder` that fills 2 histograms is removed. -Why? Because `phlex` does not yet have a histogramming facility like `TFileService` -yet. Also those histograms were booked in `beginJob`. That entire method -was removed. Booking the histograms was the only thing that was done in `beginJob`. -I'm not sure how to support `beginJob` activity in `phlex` either. - -Dropped `fEventCount`. It was not used. Seems like not a good thing -to keep track of in a `phlex` algorithm anyway. Reconstruction algorithms -should not depend on event count. Also removed the related count argument -from the findHitCandidates method of ICandidateHitFinder and its implementations. -It was not used in CandHitStandard anyway. - -MessageLogger calls are commented out for now but not deleted. Eventually -we will want to replace those with calls to whatever logging system `phlex` +Why? Because `phlex` does not yet have a histogramming facility like +`TFileService` yet. Also those histograms were booked in `beginJob`. That +entire method was removed. Booking the histograms was the only thing that was +done in `beginJob`. I'm not sure how to support `beginJob` activity in `phlex` +either. + +Dropped `fEventCount`. It was not used. Seems like not a good thing to keep +track of in a `phlex` algorithm anyway. Reconstruction algorithms should not +depend on event count. Also removed the related count argument from the +findHitCandidates method of ICandidateHitFinder and its implementations. It +was not used in CandHitStandard anyway. + +MessageLogger calls are commented out for now but not deleted. Eventually we +will want to replace those with calls to whatever logging system `phlex` eventually uses or delete those lines entirely. -For now, calls to the Geometry service are commented out but not deleted. -The current plan is to replace those with a `phlex` provider that provides the geometry -data as data products and these providers will be scheduled by the flow graph. -In the main algorithm the geometry is used to get the plane number for each -wire. For now, I just hard coded the plane number to 0. The signal type in -the `recob::Hit` objects is also from the geometry and for now always set -to 0. Similarly, the `WireID` in the `recob::Hit` objects is from the geometry -and for now set to a default value. `FillOutHitParameterVector` depends -on the geometry also, but it only checks the size of a configuration -vector against the number of planes. If this is skipped but the -configuration vector is sized to match the number of planes, then things -should still work ok. For now, I just skip FillOutHitParameterVector. - -I copied in the content of the `TMath::Gaus` function from ROOT to -avoid depending on ROOT in `phlex` code for now (slightly edited). -Are we allowed to depend on ROOT in `phlex` algorithm code? That could -easily be restored if the build system allows it. - -# Testing - -This was tested by running `GausHitFinder` with the `art` framework and running the modified `GausHitFinder` with the `phlex` framework. The two processes used identical input (the `std::vector`). The output `std::vector` objects were compared using a text file printed during each process. The output was identical except for the following: - -Two output data members of `Hit` were ignored because they depend on the `Geometry` and that is not implemented yet for `phlex`. In the `phlex` version, these data members are filled with default values. -``` +For now, calls to the Geometry service are commented out but not deleted. The +current plan is to replace those with a `phlex` provider that provides the +geometry data as data products and these providers will be scheduled by the +flow graph. In the main algorithm the geometry is used to get the plane number +for each wire. For now, I just hard coded the plane number to 0. The signal +type in the `recob::Hit` objects is also from the geometry and for now always +set to 0. Similarly, the `WireID` in the `recob::Hit` objects is from the +geometry and for now set to a default value. `FillOutHitParameterVector` +depends on the geometry also, but it only checks the size of a configuration +vector against the number of planes. If this is skipped but the configuration +vector is sized to match the number of planes, then things should still work +ok. For now, I just skip FillOutHitParameterVector. + +I copied in the content of the `TMath::Gaus` function from ROOT to avoid +depending on ROOT in `phlex` code for now (slightly edited). Are we allowed to +depend on ROOT in `phlex` algorithm code? That could easily be restored if the +build system allows it. + +## Testing + +This was tested by running `GausHitFinder` with the `art` framework and +running the modified `GausHitFinder` with the `phlex` framework. The two +processes used identical input (the `std::vector`). The output +`std::vector` objects were compared using a text file printed +during each process. The output was identical except for the following: + +Two output data members of `Hit` were ignored because they depend on the +`Geometry` and that is not implemented yet for `phlex`. In the `phlex` +version, these data members are filled with default values. + +```cpp geo::SigType_t fSignalType; ///< signal type for the plane of the hit geo::WireID fWireID; ///< WireID for the hit (Cryostat, TPC, Plane, Wire) ``` -This was done for both `phlex` versions of GausHitFinder. The output of both are identical with each other and with the `art` version. +This was done for both `phlex` versions of GausHitFinder. The output of both +are identical with each other and with the `art` version. -The `phlex` process was run with multithreading and that causes the order of `Hit` objects to vary from one execution to the next and also the order of events to vary. In the comparison the `Hit` objects were sorted and we had to be careful to compare matching events. +The `phlex` process was run with multithreading and that causes the order of +`Hit` objects to vary from one execution to the next and also the order of +events to vary. In the comparison the `Hit` objects were sorted and we had to +be careful to compare matching events. -Contact the `phlex` group if you are interested in details related -to running `art` side of this test or want the input files (or -to regenerate them). +Contact the `phlex` group if you are interested in details related to running +`art` side of this test or want the input files (or to regenerate them). diff --git a/migration/gauss_hit_finder/compare_hits.py b/migration/gauss_hit_finder/compare_hits.py index 742bc2b..69303d8 100644 --- a/migration/gauss_hit_finder/compare_hits.py +++ b/migration/gauss_hit_finder/compare_hits.py @@ -3,6 +3,7 @@ import sys + def compare_files(reference, test): # read the contents of both files into lists of lines # OK because the files are small, a few MB @@ -12,8 +13,10 @@ def compare_files(reference, test): test_lines = f.readlines() if len(ref_lines) != len(test_lines): - print(f"FAIL: Line count differs: {reference} has {len(ref_lines)} lines, " - f"{test} has {len(test_lines)} lines") + print( + f"FAIL: Line count differs: {reference} has {len(ref_lines)} lines, " + f"{test} has {len(test_lines)} lines" + ) return False total = len(ref_lines) diff --git a/migration/gauss_hit_finder/copied_from_larsoft_minor_edits/.clang-format b/migration/gauss_hit_finder/copied_from_larsoft_minor_edits/.clang-format new file mode 100644 index 0000000..e384528 --- /dev/null +++ b/migration/gauss_hit_finder/copied_from_larsoft_minor_edits/.clang-format @@ -0,0 +1 @@ +DisableFormat: true diff --git a/migration/gauss_hit_finder/find_hits_with_gaussians.cpp b/migration/gauss_hit_finder/find_hits_with_gaussians.cpp index 5139d05..5509e16 100644 --- a/migration/gauss_hit_finder/find_hits_with_gaussians.cpp +++ b/migration/gauss_hit_finder/find_hits_with_gaussians.cpp @@ -14,30 +14,36 @@ #include "tbb/concurrent_vector.h" #include "tbb/parallel_for.h" -#include "find_hits_with_gaussians.hpp" #include "copied_from_larsoft_minor_edits/ICandidateHitFinder.h" +#include "find_hits_with_gaussians.hpp" namespace { // This is an edited copy of the TMath::Gaus function from ROOT, since we // don't want to depend on ROOT in this example. - double Gaus(double x, double mean, double sigma, bool norm) { - if (sigma == 0) return 1.e30; - double arg = (x-mean)/sigma; + double Gaus(double x, double mean, double sigma, bool norm) + { + if (sigma == 0) + return 1.e30; + double arg = (x - mean) / sigma; // for |arg| > 39 result is zero in double precision - if (arg < -39.0 || arg > 39.0) return 0.0; - double res = std::exp(-0.5*arg*arg); - if (!norm) return res; - return res/(2.50662827463100024*sigma); //sqrt(2*Pi)=2.50662827463100024 + if (arg < -39.0 || arg > 39.0) + return 0.0; + double res = std::exp(-0.5 * arg * arg); + if (!norm) + return res; + return res / (2.50662827463100024 * sigma); //sqrt(2*Pi)=2.50662827463100024 } } namespace examples { - std::vector find_hits_with_gaussians(find_hits_with_gaussians_cfg const& cfg, - std::vector const& wires, - std::vector> const& cand_hit_standard, - PeakFitterMrqdt const& peak_fitter_mrqdt, - HitFilterAlg const& hit_filter_alg) { + std::vector find_hits_with_gaussians( + find_hits_with_gaussians_cfg const& cfg, + std::vector const& wires, + std::vector> const& cand_hit_standard, + PeakFitterMrqdt const& peak_fitter_mrqdt, + HitFilterAlg const& hit_filter_alg) + { std::cout << "Finding hits with Gaussians." << std::endl; @@ -78,7 +84,7 @@ namespace examples { charge += peakAmp * Gaus(sigPos, peakMean, peakWidth, false); return charge; }; - + //############################## //### Looping over the wires ### //############################## @@ -115,13 +121,13 @@ namespace examples { // ################################################# // ### Set up to loop over ROI's for this wire ### // ################################################# - const recob::Wire::RegionsOfInterest_t& signalROI = wire->SignalROI(); + recob::Wire::RegionsOfInterest_t const& signalROI = wire->SignalROI(); tbb::parallel_for( static_cast(0), signalROI.n_ranges(), [&](size_t rangeIter) { - const auto& range = signalROI.range(rangeIter); + auto const& range = signalROI.range(rangeIter); // ROI start time raw::TDCtick_t roiFirstBinTick = range.begin_index(); @@ -132,8 +138,7 @@ namespace examples { examples::ICandidateHitFinder::HitCandidateVec hitCandidateVec; examples::ICandidateHitFinder::MergeHitCandidateVec mergedCandidateHitVec; - cand_hit_standard.at(plane)->findHitCandidates( - range, 0, channel, hitCandidateVec); + cand_hit_standard.at(plane)->findHitCandidates(range, 0, channel, hitCandidateVec); cand_hit_standard.at(plane)->MergeHitCandidates( range, hitCandidateVec, mergedCandidateHitVec); @@ -148,7 +153,8 @@ namespace examples { // ### Putting in a protection in case things went wrong ### // ### In the end, this primarily catches the case where ### // ### a fake pulse is at the start of the ROI ### - if (endT - startT < 5) continue; + if (endT - startT < 5) + continue; // ####################################################### // ### Clearing the parameter vector for the new Pulse ### @@ -187,7 +193,8 @@ namespace examples { // ### depend on the fhicl parameter fLongPulseWidth ### // ### Also do this if chi^2 is too large ### // ####################################################### - if (mergedCands.size() > cfg.max_multi_hit || nGausForFit * chi2PerNDF > cfg.chi2_ndf) { + if (mergedCands.size() > cfg.max_multi_hit || + nGausForFit * chi2PerNDF > cfg.chi2_ndf) { int longPulseWidth = cfg.long_pulse_width_vec.at(plane); int nHitsThisPulse = (endT - startT) / longPulseWidth; @@ -196,7 +203,8 @@ namespace examples { longPulseWidth = (endT - startT) / nHitsThisPulse; } - if (nHitsThisPulse * longPulseWidth < endT - startT) nHitsThisPulse++; + if (nHitsThisPulse * longPulseWidth < endT - startT) + nHitsThisPulse++; int firstTick = startT; int lastTick = std::min(firstTick + longPulseWidth, endT); @@ -248,7 +256,7 @@ namespace examples { float nsigmaADC(2.0); float newright(0); float newleft(0); - for (const auto& peakParams : peakParamsVec) { + for (auto const& peakParams : peakParamsVec) { // Extract values for this hit float peakAmp = peakParams.peakAmplitude; float peakMean = peakParams.peakCenter; @@ -292,8 +300,8 @@ namespace examples { float charge = chargeFunc(peakMean, peakAmp, peakWidth, cfg.area_norms_vec[plane], startT, endT); ; - float chargeErr = - std::sqrt(std::numbers::pi) * (peakAmpErr * peakWidthErr + peakWidthErr * peakAmpErr); + float chargeErr = std::sqrt(std::numbers::pi) * + (peakAmpErr * peakWidthErr + peakWidthErr * peakAmpErr); // ### limits for getting sums std::vector::const_iterator sumStartItr = range.begin() + startT; @@ -326,46 +334,52 @@ namespace examples { } //protection to avoid negative ranges - if (newright - newleft < 0) continue; + if (newright - newleft < 0) + continue; //avoid ranges out of ROI if it happens - if (HitsumStartItr < sumStartItr) HitsumStartItr = sumStartItr; + if (HitsumStartItr < sumStartItr) + HitsumStartItr = sumStartItr; - if (HitsumEndItr > sumEndItr) HitsumEndItr = sumEndItr; + if (HitsumEndItr > sumEndItr) + HitsumEndItr = sumEndItr; - if (HitsumStartItr > HitsumEndItr) continue; + if (HitsumStartItr > HitsumEndItr) + continue; // ### Sum of ADC counts double ROIsumADC = std::accumulate(sumStartItr, sumEndItr, 0.); double HitsumADC = std::accumulate(HitsumStartItr, HitsumEndItr, 0.); - recob::Hit hit(wire->Channel(), - startT + roiFirstBinTick, - endT + roiFirstBinTick, - peakMean + roiFirstBinTick, - peakMeanErr, - peakWidth, - peakAmp, - peakAmpErr, - ROIsumADC, - HitsumADC, - charge, - chargeErr, - nGausForFit, - numHits, - chi2PerNDF, - NDF, - wire->View(), - // Geometry system for Phlex is not yet implemented, - // so we just set signal type to 0 (kInduction) for now. - geo::kInduction, - // art::ServiceHandle()->Get().SignalType(wire.Channel()), - // wid also comes from the Geometry, which is not yet implemented, so we just set - // it to a default value for now. - geo::WireID()); - // wid); - - if (cfg.filter_hits) filteredHitVec.push_back(hit); + recob::Hit hit( + wire->Channel(), + startT + roiFirstBinTick, + endT + roiFirstBinTick, + peakMean + roiFirstBinTick, + peakMeanErr, + peakWidth, + peakAmp, + peakAmpErr, + ROIsumADC, + HitsumADC, + charge, + chargeErr, + nGausForFit, + numHits, + chi2PerNDF, + NDF, + wire->View(), + // Geometry system for Phlex is not yet implemented, + // so we just set signal type to 0 (kInduction) for now. + geo::kInduction, + // art::ServiceHandle()->Get().SignalType(wire.Channel()), + // wid also comes from the Geometry, which is not yet implemented, so we just set + // it to a default value for now. + geo::WireID()); + // wid); + + if (cfg.filter_hits) + filteredHitVec.push_back(hit); // This loop will store ALL hits hitstruct tmp{std::move(hit)}; @@ -388,7 +402,7 @@ namespace examples { // is in descending order of peak height, not ascending.) std::sort(filteredHitVec.begin(), filteredHitVec.end(), - [](const auto& left, const auto& right) { + [](auto const& left, auto const& right) { return left.PeakAmplitude() > right.PeakAmplitude(); }); @@ -408,7 +422,7 @@ namespace examples { std::vector::iterator smallHitItr = std::find_if(filteredHitVec.begin(), filteredHitVec.end(), - [largestPH, threshold](const auto& hit) { + [largestPH, threshold](auto const& hit) { return hit.PeakAmplitude() < 8. && hit.PeakAmplitude() / largestPH < threshold; }); @@ -420,13 +434,13 @@ namespace examples { // Resort in time order std::sort(filteredHitVec.begin(), filteredHitVec.end(), - [](const auto& left, const auto& right) { + [](auto const& left, auto const& right) { return left.PeakTime() < right.PeakTime(); }); } // Copy the hits we want to keep to the filtered hit collection - for (const auto& filteredHit : filteredHitVec) { + for (auto const& filteredHit : filteredHitVec) { if (!cfg.filter_hits || hit_filter_alg.IsGoodHit(filteredHit)) { hitstruct tmp{std::move(filteredHit)}; filthitstruct_vec.push_back(std::move(tmp)); @@ -434,10 +448,10 @@ namespace examples { } } } //<---End loop over merged candidate hits - } //<---End looping over ROI's - ); //end tbb parallel for - } //<---End looping over all the wires - ); //end tbb parallel for + } //<---End looping over ROI's + ); //end tbb parallel for + } //<---End looping over all the wires + ); //end tbb parallel for std::vector hits; diff --git a/migration/gauss_hit_finder/find_hits_with_gaussians.hpp b/migration/gauss_hit_finder/find_hits_with_gaussians.hpp index f1eefd5..c40c109 100644 --- a/migration/gauss_hit_finder/find_hits_with_gaussians.hpp +++ b/migration/gauss_hit_finder/find_hits_with_gaussians.hpp @@ -1,5 +1,5 @@ -#ifndef PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_HPP -#define PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_HPP +#ifndef MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_HPP +#define MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_HPP // See REAMDME.md for some general comments about this example. @@ -36,7 +36,6 @@ #include "copied_from_larsoft_minor_edits/PeakFitterMrqdt.h" #include "copied_from_larsoft_minor_edits/Wire.h" - namespace examples { struct find_hits_with_gaussians_cfg { @@ -44,22 +43,22 @@ namespace examples { std::vector long_max_hits_vec; /// long_pulse_width_vec; /// - area_norms_vec; /// area_norms_vec; /// pulse_height_cuts; std::vector pulse_width_cuts; std::vector pulse_ratio_cuts; }; - std::vector find_hits_with_gaussians(find_hits_with_gaussians_cfg const& cfg, - std::vector const& wires, - std::vector> const& cand_hit_standard, - PeakFitterMrqdt const& peak_fitter_mrqdt, - HitFilterAlg const& hit_filter_alg); + std::vector find_hits_with_gaussians( + find_hits_with_gaussians_cfg const& cfg, + std::vector const& wires, + std::vector> const& cand_hit_standard, + PeakFitterMrqdt const& peak_fitter_mrqdt, + HitFilterAlg const& hit_filter_alg); } -#endif // PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_HPP +#endif // MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_HPP diff --git a/migration/gauss_hit_finder/find_hits_with_gaussians_design1.cpp b/migration/gauss_hit_finder/find_hits_with_gaussians_design1.cpp index f26c397..777d3de 100644 --- a/migration/gauss_hit_finder/find_hits_with_gaussians_design1.cpp +++ b/migration/gauss_hit_finder/find_hits_with_gaussians_design1.cpp @@ -14,9 +14,9 @@ #include "tbb/parallel_for.h" -#include "copied_from_larsoft_minor_edits/geo_types.h" // geo::View_t, geo::SignalType, geo::WireID #include "copied_from_larsoft_minor_edits/ICandidateHitFinder.h" -#include "copied_from_larsoft_minor_edits/RawTypes.h" // raw::ChannelID_t +#include "copied_from_larsoft_minor_edits/RawTypes.h" // raw::ChannelID_t +#include "copied_from_larsoft_minor_edits/geo_types.h" // geo::View_t, geo::SignalType, geo::WireID #include "find_hits_with_gaussians_design1.hpp" namespace { @@ -39,8 +39,7 @@ namespace { namespace examples { - unfold_wire_vector_design1::unfold_wire_vector_design1( - std::vector const& wires) : + unfold_wire_vector_design1::unfold_wire_vector_design1(std::vector const& wires) : begin_{wires.begin()}, end_{wires.end()} { // Probably eventually delete the following line @@ -53,7 +52,10 @@ namespace examples { return begin_; } - bool unfold_wire_vector_design1::predicate(const_iterator current) const { return current != end_; } + bool unfold_wire_vector_design1::predicate(const_iterator current) const + { + return current != end_; + } std::pair unfold_wire_vector_design1::unfold(const_iterator current) const diff --git a/migration/gauss_hit_finder/find_hits_with_gaussians_design1.hpp b/migration/gauss_hit_finder/find_hits_with_gaussians_design1.hpp index 5e56def..06103f6 100644 --- a/migration/gauss_hit_finder/find_hits_with_gaussians_design1.hpp +++ b/migration/gauss_hit_finder/find_hits_with_gaussians_design1.hpp @@ -1,5 +1,5 @@ -#ifndef PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP -#define PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP +#ifndef MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP +#define MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP // See REAMDME.md for some general comments about this example. @@ -46,7 +46,6 @@ namespace examples { class unfold_wire_vector_design1 { public: - explicit unfold_wire_vector_design1(std::vector const& wires); using const_iterator = std::vector::const_iterator; @@ -69,25 +68,25 @@ namespace examples { std::vector long_max_hits_vec; /// long_pulse_width_vec; /// - area_norms_vec; /// area_norms_vec; /// pulse_height_cuts; std::vector pulse_width_cuts; std::vector pulse_ratio_cuts; }; - tbb::concurrent_vector find_hits_with_gaussians_design1(find_hits_with_gaussians_design1_cfg const& cfg, - recob::Wire const& wire, - std::vector> const& cand_hit_standard, - PeakFitterMrqdt const& peak_fitter_mrqdt, - HitFilterAlg const& hit_filter_alg); + tbb::concurrent_vector find_hits_with_gaussians_design1( + find_hits_with_gaussians_design1_cfg const& cfg, + recob::Wire const& wire, + std::vector> const& cand_hit_standard, + PeakFitterMrqdt const& peak_fitter_mrqdt, + HitFilterAlg const& hit_filter_alg); // Third, define the fold, fills the output vector of hits void fold_hits_into_vector_design1(std::vector& hits, tbb::concurrent_vector const& hits_from_wire); } -#endif // PHLEX_EXAMPLES_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP +#endif // MIGRATION_GAUSS_HIT_FINDER_FIND_HITS_WITH_GAUSSIANS_DESIGN1_HPP diff --git a/migration/gauss_hit_finder/print_hits_to_file.cpp b/migration/gauss_hit_finder/print_hits_to_file.cpp index a7f5a18..26dd01e 100644 --- a/migration/gauss_hit_finder/print_hits_to_file.cpp +++ b/migration/gauss_hit_finder/print_hits_to_file.cpp @@ -13,8 +13,8 @@ namespace examples { - void print_hits_to_file(int cell_id, - std::vector const& input_hits) { + void print_hits_to_file(int cell_id, std::vector const& input_hits) + { std::vector hits = input_hits; @@ -23,7 +23,7 @@ namespace examples { // the output from the art version of GausHitFinder. The parallel_for // loops and unfold-transform-fold execution map both scramble the // order and make comparisons difficult. - std::sort(hits.begin(), hits.end(), [](const recob::Hit& a, const recob::Hit& b) { + std::sort(hits.begin(), hits.end(), [](recob::Hit const& a, recob::Hit const& b) { if (a.PeakTime() != b.PeakTime()) { return a.PeakTime() < b.PeakTime(); } @@ -36,8 +36,7 @@ namespace examples { return a.EndTick() < b.EndTick(); }); - std::string filename = std::string("hits_") + - std::to_string(cell_id) + ".txt"; + std::string filename = std::string("hits_") + std::to_string(cell_id) + ".txt"; auto file = fmt::output_file(filename); file.print("Contents of a std::vector\n\n"); diff --git a/migration/gauss_hit_finder/print_hits_to_file.hpp b/migration/gauss_hit_finder/print_hits_to_file.hpp index 326686e..669ead0 100644 --- a/migration/gauss_hit_finder/print_hits_to_file.hpp +++ b/migration/gauss_hit_finder/print_hits_to_file.hpp @@ -1,5 +1,5 @@ -#ifndef PHLEX_EXAMPLES_PRINT_HITS_HPP -#define PHLEX_EXAMPLES_PRINT_HITS_HPP +#ifndef MIGRATION_GAUSS_HIT_FINDER_PRINT_HITS_TO_FILE_HPP +#define MIGRATION_GAUSS_HIT_FINDER_PRINT_HITS_TO_FILE_HPP #include @@ -7,8 +7,7 @@ namespace examples { - void print_hits_to_file(int cell_id, - std::vector const& input_hits); + void print_hits_to_file(int cell_id, std::vector const& input_hits); } -#endif // PHLEX_EXAMPLES_PRINT_HITS_HPP +#endif // MIGRATION_GAUSS_HIT_FINDER_PRINT_HITS_TO_FILE_HPP diff --git a/migration/gauss_hit_finder/register_find_hits_with_gaussians.cpp b/migration/gauss_hit_finder/register_find_hits_with_gaussians.cpp index e2a6ded..3084cf9 100644 --- a/migration/gauss_hit_finder/register_find_hits_with_gaussians.cpp +++ b/migration/gauss_hit_finder/register_find_hits_with_gaussians.cpp @@ -12,8 +12,8 @@ #include "copied_from_larsoft_minor_edits/PeakFitterMrqdt.h" #include "copied_from_larsoft_minor_edits/Wire.h" -#include "phlex/module.hpp" #include "find_hits_with_gaussians.hpp" +#include "phlex/module.hpp" using namespace phlex; @@ -31,8 +31,7 @@ PHLEX_REGISTER_ALGORITHMS(m, config) .chi2_ndf = config.get("chi2_ndf"), .pulse_height_cuts = config.get>("pulse_height_cuts"), .pulse_width_cuts = config.get>("pulse_width_cuts"), - .pulse_ratio_cuts = config.get>("pulse_ratio_cuts") - }; + .pulse_ratio_cuts = config.get>("pulse_ratio_cuts")}; std::vector> cand_hit_standard_vec; @@ -41,12 +40,12 @@ PHLEX_REGISTER_ALGORITHMS(m, config) for (auto const& key : finder_configs.keys()) { auto finder_config = finder_configs.get(key); auto hit_finder = std::make_shared( - examples::CandHitStandardCfg{ - .fRoiThreshold = finder_config.get("roiThreshold") - }); + examples::CandHitStandardCfg{.fRoiThreshold = finder_config.get("roiThreshold")}); unsigned int plane = finder_config.get("Plane"); if (plane >= cand_hit_standard_vec.size()) { - std::cerr << "Error: plane number " << plane << " is out of range for cand_hit_standard_vec of size " << cand_hit_standard_vec.size() << std::endl; + std::cerr << "Error: plane number " << plane + << " is out of range for cand_hit_standard_vec of size " + << cand_hit_standard_vec.size() << std::endl; throw std::runtime_error("Invalid plane number"); } cand_hit_standard_vec[plane] = std::move(hit_finder); @@ -55,35 +54,27 @@ PHLEX_REGISTER_ALGORITHMS(m, config) std::shared_ptr peak_fitter_mrqdt; auto fitter_config = config.get("peak_fitter_mrqdt_config"); peak_fitter_mrqdt = std::make_shared( - examples::PeakFitterMrqdtCfg{ - .fMinWidth = fitter_config.get("min_width"), - .fMaxWidthMult = fitter_config.get("max_width_mult"), - .fPeakRange = fitter_config.get("peak_range_fact"), - .fAmpRange = fitter_config.get("peak_amp_range") - }); - + examples::PeakFitterMrqdtCfg{.fMinWidth = fitter_config.get("min_width"), + .fMaxWidthMult = fitter_config.get("max_width_mult"), + .fPeakRange = fitter_config.get("peak_range_fact"), + .fAmpRange = fitter_config.get("peak_amp_range")}); std::shared_ptr hit_filter_alg; auto filter_config = config.get("hit_filter_alg_config"); - hit_filter_alg = std::make_shared( - examples::HitFilterAlgCfg{ - .fMinPulseHeight = filter_config.get>("min_pulse_height"), - .fMinPulseSigma = filter_config.get>("min_pulse_sigma") - }); + hit_filter_alg = std::make_shared(examples::HitFilterAlgCfg{ + .fMinPulseHeight = filter_config.get>("min_pulse_height"), + .fMinPulseSigma = filter_config.get>("min_pulse_sigma")}); - m.transform("find_hits_with_gaussians", - [cfg = std::move(cfg), - cand_hit_standard_vec = std::move(cand_hit_standard_vec), - peak_fitter_mrqdt = std::move(peak_fitter_mrqdt), - hit_filter_alg = std::move(hit_filter_alg)] - (std::vector const& wires) { - return examples::find_hits_with_gaussians(cfg, - wires, - cand_hit_standard_vec, - *peak_fitter_mrqdt, - *hit_filter_alg); - }, - concurrency::unlimited) + m.transform( + "find_hits_with_gaussians", + [cfg = std::move(cfg), + cand_hit_standard_vec = std::move(cand_hit_standard_vec), + peak_fitter_mrqdt = std::move(peak_fitter_mrqdt), + hit_filter_alg = std::move(hit_filter_alg)](std::vector const& wires) { + return examples::find_hits_with_gaussians( + cfg, wires, cand_hit_standard_vec, *peak_fitter_mrqdt, *hit_filter_alg); + }, + concurrency::unlimited) .input_family(product_selector{.creator = "wires", .layer = layer, .suffix = ""}) .output_product_suffixes("hits"); } diff --git a/migration/gauss_hit_finder/register_find_hits_with_gaussians_cell_id.cpp b/migration/gauss_hit_finder/register_find_hits_with_gaussians_cell_id.cpp index 71b2c88..76e071b 100644 --- a/migration/gauss_hit_finder/register_find_hits_with_gaussians_cell_id.cpp +++ b/migration/gauss_hit_finder/register_find_hits_with_gaussians_cell_id.cpp @@ -10,7 +10,6 @@ PHLEX_REGISTER_PROVIDERS(m, config) { auto const layer = config.get("layer"); - m.provide("cell_id", [](data_cell_index const& id) -> int { - return id.number(); - }).output_product("cell_info", "", experimental::identifier{layer}); + m.provide("cell_id", [](data_cell_index const& id) -> int { return id.number(); }) + .output_product("cell_info", "", experimental::identifier{layer}); } diff --git a/migration/gauss_hit_finder/register_find_hits_with_gaussians_design1.cpp b/migration/gauss_hit_finder/register_find_hits_with_gaussians_design1.cpp index cd8ca06..0f8270c 100644 --- a/migration/gauss_hit_finder/register_find_hits_with_gaussians_design1.cpp +++ b/migration/gauss_hit_finder/register_find_hits_with_gaussians_design1.cpp @@ -41,8 +41,7 @@ PHLEX_REGISTER_ALGORITHMS(m, config) .chi2_ndf = config.get("chi2_ndf"), .pulse_height_cuts = config.get>("pulse_height_cuts"), .pulse_width_cuts = config.get>("pulse_width_cuts"), - .pulse_ratio_cuts = config.get>("pulse_ratio_cuts") - }; + .pulse_ratio_cuts = config.get>("pulse_ratio_cuts")}; std::vector> cand_hit_standard_vec; @@ -51,12 +50,12 @@ PHLEX_REGISTER_ALGORITHMS(m, config) for (auto const& key : finder_configs.keys()) { auto finder_config = finder_configs.get(key); auto hit_finder = std::make_shared( - examples::CandHitStandardCfg{ - .fRoiThreshold = finder_config.get("roiThreshold") - }); + examples::CandHitStandardCfg{.fRoiThreshold = finder_config.get("roiThreshold")}); unsigned int plane = finder_config.get("Plane"); if (plane >= cand_hit_standard_vec.size()) { - std::cerr << "Error: plane number " << plane << " is out of range for cand_hit_standard_vec of size " << cand_hit_standard_vec.size() << std::endl; + std::cerr << "Error: plane number " << plane + << " is out of range for cand_hit_standard_vec of size " + << cand_hit_standard_vec.size() << std::endl; throw std::runtime_error("Invalid plane number"); } cand_hit_standard_vec[plane] = std::move(hit_finder); @@ -65,37 +64,34 @@ PHLEX_REGISTER_ALGORITHMS(m, config) std::shared_ptr peak_fitter_mrqdt; auto fitter_config = config.get("peak_fitter_mrqdt_config"); peak_fitter_mrqdt = std::make_shared( - examples::PeakFitterMrqdtCfg{ - .fMinWidth = fitter_config.get("min_width"), - .fMaxWidthMult = fitter_config.get("max_width_mult"), - .fPeakRange = fitter_config.get("peak_range_fact"), - .fAmpRange = fitter_config.get("peak_amp_range") - }); - + examples::PeakFitterMrqdtCfg{.fMinWidth = fitter_config.get("min_width"), + .fMaxWidthMult = fitter_config.get("max_width_mult"), + .fPeakRange = fitter_config.get("peak_range_fact"), + .fAmpRange = fitter_config.get("peak_amp_range")}); std::shared_ptr hit_filter_alg; auto filter_config = config.get("hit_filter_alg_config"); - hit_filter_alg = std::make_shared( - examples::HitFilterAlgCfg{ - .fMinPulseHeight = filter_config.get>("min_pulse_height"), - .fMinPulseSigma = filter_config.get>("min_pulse_sigma") - }); + hit_filter_alg = std::make_shared(examples::HitFilterAlgCfg{ + .fMinPulseHeight = filter_config.get>("min_pulse_height"), + .fMinPulseSigma = filter_config.get>("min_pulse_sigma")}); - m.transform("find_hits_with_gaussians_design1", - [cfg = std::move(cfg), - cand_hit_standard_vec = std::move(cand_hit_standard_vec), - peak_fitter_mrqdt = std::move(peak_fitter_mrqdt), - hit_filter_alg = std::move(hit_filter_alg)] - (recob::Wire const& wire) { - return examples::find_hits_with_gaussians_design1(cfg, - wire, - cand_hit_standard_vec, - *peak_fitter_mrqdt, - *hit_filter_alg); - }, - concurrency::unlimited) - .input_family(product_selector{.creator = "unfold_wire_vector_design1", .layer = layer_unfold_output}); + m.transform( + "find_hits_with_gaussians_design1", + [cfg = std::move(cfg), + cand_hit_standard_vec = std::move(cand_hit_standard_vec), + peak_fitter_mrqdt = std::move(peak_fitter_mrqdt), + hit_filter_alg = std::move(hit_filter_alg)](recob::Wire const& wire) { + return examples::find_hits_with_gaussians_design1( + cfg, wire, cand_hit_standard_vec, *peak_fitter_mrqdt, *hit_filter_alg); + }, + concurrency::unlimited) + .input_family( + product_selector{.creator = "unfold_wire_vector_design1", .layer = layer_unfold_output}); - m.fold("fold_hits_into_vector_design1", examples::fold_hits_into_vector_design1, concurrency::serial, layer_unfold_input) - .input_family(product_selector{.creator = "find_hits_with_gaussians_design1", .layer = layer_unfold_output}); + m.fold("fold_hits_into_vector_design1", + examples::fold_hits_into_vector_design1, + concurrency::serial, + layer_unfold_input) + .input_family(product_selector{.creator = "find_hits_with_gaussians_design1", + .layer = layer_unfold_output}); } diff --git a/migration/gauss_hit_finder/register_print_hits_to_file.cpp b/migration/gauss_hit_finder/register_print_hits_to_file.cpp index a605fe4..7e359fb 100644 --- a/migration/gauss_hit_finder/register_print_hits_to_file.cpp +++ b/migration/gauss_hit_finder/register_print_hits_to_file.cpp @@ -17,7 +17,6 @@ PHLEX_REGISTER_ALGORITHMS(m, config) auto const layer = config.get("layer"); m.observe("print_hits_to_file", examples::print_hits_to_file, concurrency::unlimited) - .input_family( - product_selector{.creator = "cell_info", .layer = layer}, - product_selector{.creator = creator, .layer = layer}); + .input_family(product_selector{.creator = "cell_info", .layer = layer}, + product_selector{.creator = creator, .layer = layer}); } diff --git a/migration/gauss_hit_finder/test_find_hits_with_gaussians.jsonnet b/migration/gauss_hit_finder/test_find_hits_with_gaussians.jsonnet index b0a16c5..633b71c 100644 --- a/migration/gauss_hit_finder/test_find_hits_with_gaussians.jsonnet +++ b/migration/gauss_hit_finder/test_find_hits_with_gaussians.jsonnet @@ -33,7 +33,7 @@ area_method: 0, area_norms_vec: [13.25, 13.25, 13.25], chi2_ndf: 50.0, - pulse_height_cuts: [3.0, 3.0, 3.0], + pulse_height_cuts: [3.0, 3.0, 3.0], pulse_width_cuts: [2.0, 1.5, 1.0], pulse_ratio_cuts: [0.35, 0.40, 0.20], cand_hit_standard_configs: { diff --git a/migration/gauss_hit_finder/test_find_hits_with_gaussians_design1.jsonnet b/migration/gauss_hit_finder/test_find_hits_with_gaussians_design1.jsonnet index 8ebce1c..8d26d11 100644 --- a/migration/gauss_hit_finder/test_find_hits_with_gaussians_design1.jsonnet +++ b/migration/gauss_hit_finder/test_find_hits_with_gaussians_design1.jsonnet @@ -34,7 +34,7 @@ area_method: 0, area_norms_vec: [13.25, 13.25, 13.25], chi2_ndf: 50.0, - pulse_height_cuts: [3.0, 3.0, 3.0], + pulse_height_cuts: [3.0, 3.0, 3.0], pulse_width_cuts: [2.0, 1.5, 1.0], pulse_ratio_cuts: [0.35, 0.40, 0.20], cand_hit_standard_configs: { diff --git a/migration/gauss_hit_finder/wire_serialization.cpp b/migration/gauss_hit_finder/wire_serialization.cpp index 5eb6241..a5bb2f4 100644 --- a/migration/gauss_hit_finder/wire_serialization.cpp +++ b/migration/gauss_hit_finder/wire_serialization.cpp @@ -1,129 +1,136 @@ +#include "wire_serialization.hpp" +#include "copied_from_larsoft_minor_edits/sparse_vector.h" #include #include -#include "copied_from_larsoft_minor_edits/sparse_vector.h" -#include "wire_serialization.hpp" -bool write_wires_to_file(std::vector const& wires, - std::string const& filename) { - std::ofstream out(filename, std::ios::binary); - if (!out) { - throw std::runtime_error("Failed to open file for writing: " + filename); +bool write_wires_to_file(std::vector const& wires, std::string const& filename) +{ + std::ofstream out(filename, std::ios::binary); + if (!out) { + throw std::runtime_error("Failed to open file for writing: " + filename); + } + + // Write number of wires + size_t num_wires = wires.size(); + out.write(reinterpret_cast(&num_wires), sizeof(num_wires)); + + for (auto const& wire : wires) { + // Write channel ID + raw::ChannelID_t channel = wire.Channel(); + out.write(reinterpret_cast(&channel), sizeof(channel)); + + // Write view + geo::View_t view = wire.View(); + out.write(reinterpret_cast(&view), sizeof(view)); + + // Write sparse vector data + auto const& signal_roi = wire.SignalROI(); + size_t nominal_size = signal_roi.size(); + out.write(reinterpret_cast(&nominal_size), sizeof(nominal_size)); + + // Write number of ranges + size_t num_ranges = std::distance(signal_roi.begin_range(), signal_roi.end_range()); + out.write(reinterpret_cast(&num_ranges), sizeof(num_ranges)); + + // Write each range + for (auto range_it = signal_roi.begin_range(); range_it != signal_roi.end_range(); ++range_it) { + // Write range offset and last + size_t offset = range_it->begin_index(); + size_t last = range_it->end_index(); + out.write(reinterpret_cast(&offset), sizeof(offset)); + out.write(reinterpret_cast(&last), sizeof(last)); + + // Write number of values + size_t num_values = range_it->size(); + out.write(reinterpret_cast(&num_values), sizeof(num_values)); + + // Write values + if (num_values > 0) { + // Create a temporary vector to hold the values + std::vector values(range_it->begin(), range_it->end()); + out.write(reinterpret_cast(values.data()), num_values * sizeof(float)); + } } - - // Write number of wires - size_t num_wires = wires.size(); - out.write(reinterpret_cast(&num_wires), sizeof(num_wires)); - - for (const auto& wire : wires) { - // Write channel ID - raw::ChannelID_t channel = wire.Channel(); - out.write(reinterpret_cast(&channel), sizeof(channel)); - - // Write view - geo::View_t view = wire.View(); - out.write(reinterpret_cast(&view), sizeof(view)); - - // Write sparse vector data - const auto& signal_roi = wire.SignalROI(); - size_t nominal_size = signal_roi.size(); - out.write(reinterpret_cast(&nominal_size), sizeof(nominal_size)); - - // Write number of ranges - size_t num_ranges = std::distance(signal_roi.begin_range(), signal_roi.end_range()); - out.write(reinterpret_cast(&num_ranges), sizeof(num_ranges)); - - // Write each range - for (auto range_it = signal_roi.begin_range(); range_it != signal_roi.end_range(); ++range_it) { - // Write range offset and last - size_t offset = range_it->begin_index(); - size_t last = range_it->end_index(); - out.write(reinterpret_cast(&offset), sizeof(offset)); - out.write(reinterpret_cast(&last), sizeof(last)); - - // Write number of values - size_t num_values = range_it->size(); - out.write(reinterpret_cast(&num_values), sizeof(num_values)); - - // Write values - if (num_values > 0) { - // Create a temporary vector to hold the values - std::vector values(range_it->begin(), range_it->end()); - out.write(reinterpret_cast(values.data()), - num_values * sizeof(float)); - } - } - } - out.close(); - return out.good(); + } + out.close(); + return out.good(); } -std::optional> read_wires_from_file(std::string const& filename) { - std::ifstream in(filename, std::ios::binary); - if (!in) { - throw std::runtime_error("Failed to open file for reading: " + filename); +std::optional> read_wires_from_file(std::string const& filename) +{ + std::ifstream in(filename, std::ios::binary); + if (!in) { + throw std::runtime_error("Failed to open file for reading: " + filename); + } + + std::vector wires; + + // Read number of wires + size_t num_wires; + in.read(reinterpret_cast(&num_wires), sizeof(num_wires)); + if (!in) + return std::nullopt; + + wires.reserve(num_wires); + + for (size_t i = 0; i < num_wires; ++i) { + // Read channel ID + raw::ChannelID_t channel; + in.read(reinterpret_cast(&channel), sizeof(channel)); + if (!in) + return std::nullopt; + + // Read view + geo::View_t view; + in.read(reinterpret_cast(&view), sizeof(view)); + if (!in) + return std::nullopt; + + // Read sparse vector data + size_t nominal_size; + in.read(reinterpret_cast(&nominal_size), sizeof(nominal_size)); + if (!in) + return std::nullopt; + + // Read number of ranges + size_t num_ranges; + in.read(reinterpret_cast(&num_ranges), sizeof(num_ranges)); + if (!in) + return std::nullopt; + + // Create sparse vector + lar::sparse_vector signal_roi; + signal_roi.resize(nominal_size); + + // Read each range + for (size_t j = 0; j < num_ranges; ++j) { + // Read range offset and last + size_t offset, last; + in.read(reinterpret_cast(&offset), sizeof(offset)); + in.read(reinterpret_cast(&last), sizeof(last)); + if (!in) + return std::nullopt; + + // Read number of values + size_t num_values; + in.read(reinterpret_cast(&num_values), sizeof(num_values)); + if (!in) + return std::nullopt; + + // Read values + std::vector values(num_values); + if (num_values > 0) { + in.read(reinterpret_cast(values.data()), num_values * sizeof(float)); + if (!in) + return std::nullopt; + } + + // Add range to sparse vector + signal_roi.add_range(offset, values); } - std::vector wires; - - // Read number of wires - size_t num_wires; - in.read(reinterpret_cast(&num_wires), sizeof(num_wires)); - if (!in) return std::nullopt; - - wires.reserve(num_wires); - - for (size_t i = 0; i < num_wires; ++i) { - // Read channel ID - raw::ChannelID_t channel; - in.read(reinterpret_cast(&channel), sizeof(channel)); - if (!in) return std::nullopt; - - // Read view - geo::View_t view; - in.read(reinterpret_cast(&view), sizeof(view)); - if (!in) return std::nullopt; - - // Read sparse vector data - size_t nominal_size; - in.read(reinterpret_cast(&nominal_size), sizeof(nominal_size)); - if (!in) return std::nullopt; - - // Read number of ranges - size_t num_ranges; - in.read(reinterpret_cast(&num_ranges), sizeof(num_ranges)); - if (!in) return std::nullopt; - - // Create sparse vector - lar::sparse_vector signal_roi; - signal_roi.resize(nominal_size); - - // Read each range - for (size_t j = 0; j < num_ranges; ++j) { - // Read range offset and last - size_t offset, last; - in.read(reinterpret_cast(&offset), sizeof(offset)); - in.read(reinterpret_cast(&last), sizeof(last)); - if (!in) return std::nullopt; - - // Read number of values - size_t num_values; - in.read(reinterpret_cast(&num_values), sizeof(num_values)); - if (!in) return std::nullopt; - - // Read values - std::vector values(num_values); - if (num_values > 0) { - in.read(reinterpret_cast(values.data()), - num_values * sizeof(float)); - if (!in) return std::nullopt; - } - - // Add range to sparse vector - signal_roi.add_range(offset, values); - } - - // Create wire - wires.emplace_back(std::move(signal_roi), channel, view); - } - return wires; + // Create wire + wires.emplace_back(std::move(signal_roi), channel, view); + } + return wires; } diff --git a/migration/gauss_hit_finder/wire_serialization.hpp b/migration/gauss_hit_finder/wire_serialization.hpp index f038856..048147f 100644 --- a/migration/gauss_hit_finder/wire_serialization.hpp +++ b/migration/gauss_hit_finder/wire_serialization.hpp @@ -1,15 +1,14 @@ -#ifndef PHLEX_EXAMPLES_WIRE_SERIALIZATION_H -#define PHLEX_EXAMPLES_WIRE_SERIALIZATION_H +#ifndef MIGRATION_GAUSS_HIT_FINDER_WIRE_SERIALIZATION_HPP +#define MIGRATION_GAUSS_HIT_FINDER_WIRE_SERIALIZATION_HPP +#include "copied_from_larsoft_minor_edits/Wire.h" #include #include #include #include -#include "copied_from_larsoft_minor_edits/Wire.h" -bool write_wires_to_file(std::vector const& wires, - std::string const& filename); +bool write_wires_to_file(std::vector const& wires, std::string const& filename); std::optional> read_wires_from_file(std::string const& filename); -#endif // PHLEX_EXAMPLES_WIRE_SERIALIZATION_H +#endif // MIGRATION_GAUSS_HIT_FINDER_WIRE_SERIALIZATION_HPP diff --git a/migration/gauss_hit_finder/wires_source.cpp b/migration/gauss_hit_finder/wires_source.cpp index e1422f5..f862685 100644 --- a/migration/gauss_hit_finder/wires_source.cpp +++ b/migration/gauss_hit_finder/wires_source.cpp @@ -23,15 +23,15 @@ PHLEX_REGISTER_PROVIDERS(m, config) { auto const layer = config.get("layer"); - m.provide("provide_wires", [](data_cell_index const& id) -> std::vector { - std::string filename = std::string("wires_") + std::to_string(id.number()) + ".dat"; - - - if (auto wires = read_wires_from_file(filename)) { - return *wires; - } - throw std::runtime_error("Failure while reading from file: " + filename); - return {}; - }) + m.provide("provide_wires", + [](data_cell_index const& id) -> std::vector { + std::string filename = std::string("wires_") + std::to_string(id.number()) + ".dat"; + + if (auto wires = read_wires_from_file(filename)) { + return *wires; + } + throw std::runtime_error("Failure while reading from file: " + filename); + return {}; + }) .output_product("wires", "", experimental::identifier{layer}); } diff --git a/scripts/fix_header_guards.py b/scripts/fix_header_guards.py new file mode 100755 index 0000000..e9ca7a3 --- /dev/null +++ b/scripts/fix_header_guards.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +"""Check and fix header guards to enforce naming convention.""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + + +def compute_expected_guard(file_path: Path, root: Path) -> str: + """Compute expected guard macro: X_Y_HEADER_EXT.""" + rel = file_path.relative_to(root) + # X is the first subdirectory when present, Y is remaining path components + parts = [] + if len(rel.parts) > 1: + parts.append(rel.parts[0].upper().replace("-", "_")) + if len(rel.parts) > 2: + parts.extend(p.upper().replace("-", "_") for p in rel.parts[1:-1]) + parts.extend([rel.stem.upper().replace("-", "_"), rel.suffix[1:].upper()]) + return "_".join(parts) + + +def check_header_guard(file_path: Path, root: Path) -> tuple[bool, str | None]: + """Check if header guard is correct. Returns (is_valid, expected_guard).""" + content = file_path.read_text(encoding="utf-8") + lines = content.splitlines(keepends=True) + + if len(lines) < 3: + return True, None + + expected = compute_expected_guard(file_path, root) + + ifndef_idx = define_idx = None + ifndef_macro = define_macro = endif_macro = None + + for i in range(min(10, len(lines))): + if m := re.match(r"#ifndef\s+(\w+)\s*$", lines[i]): + ifndef_idx, ifndef_macro = i, m.group(1) + elif m := re.match(r"#define\s+(\w+)\s*$", lines[i]): + define_idx, define_macro = i, m.group(1) + break + + for i in range(len(lines) - 1, -1, -1): + line = lines[i].strip() + if line.startswith("#endif"): + if m := re.match(r"#endif\s*//\s*(\w+)\s*$", lines[i]): + endif_macro = m.group(1) + break + + if ifndef_idx is None or define_idx is None: + return True, None + + if ifndef_macro == define_macro == endif_macro == expected: + return True, None + + return False, expected + + +def fix_header_guard(file_path: Path, root: Path) -> bool: + """Fix header guard. Returns True if modified.""" + content = file_path.read_text(encoding="utf-8") + lines = content.splitlines(keepends=True) + + if len(lines) < 3: + return False + + expected = compute_expected_guard(file_path, root) + + ifndef_idx = define_idx = endif_idx = None + + for i in range(min(10, len(lines))): + if re.match(r"#ifndef\s+\w+\s*$", lines[i]): + ifndef_idx = i + elif re.match(r"#define\s+\w+\s*$", lines[i]): + define_idx = i + break + + for i in range(len(lines) - 1, -1, -1): + line = lines[i].strip() + if line.startswith("#endif"): + endif_idx = i + break + + if ifndef_idx is None or define_idx is None: + return False + + modified = False + if lines[ifndef_idx] != f"#ifndef {expected}\n": + lines[ifndef_idx] = f"#ifndef {expected}\n" + modified = True + if lines[define_idx] != f"#define {expected}\n": + lines[define_idx] = f"#define {expected}\n" + modified = True + if endif_idx is not None and lines[endif_idx] != f"#endif // {expected}\n": + lines[endif_idx] = f"#endif // {expected}\n" + modified = True + + if modified: + file_path.write_text("".join(lines), encoding="utf-8") + return modified + + +def main() -> None: + """Check or fix header guards in C++ header files.""" + parser = argparse.ArgumentParser(description="Check/fix header guards") + parser.add_argument("paths", nargs="+", help="Files or directories") + parser.add_argument("--check", action="store_true", help="Check only") + parser.add_argument("--root", type=Path, default=Path.cwd(), help="Root path") + args = parser.parse_args() + + root = args.root.resolve() + bad_files = [] + + for arg in args.paths: + path = Path(arg).resolve() + files = [path] if path.is_file() else [*path.rglob("*.hpp"), *path.rglob("*.h")] + for f in files: + if f.suffix not in {".hpp", ".h"}: + continue + if args.check: + valid, expected = check_header_guard(f, root) + if not valid: + bad_files.append((f, expected)) + else: + if fix_header_guard(f, root): + bad_files.append((f, None)) + + if args.check: + if bad_files: + print(f"Found {len(bad_files)} files with incorrect guards:") + for f, expected in bad_files: + print(f" {f.relative_to(root)}: expected {expected}") + sys.exit(1) + print("All header guards are correct.") + else: + if bad_files: + print(f"Fixed {len(bad_files)} files:") + for f, _ in bad_files: + print(f" {f.relative_to(root)}") + else: + print("No header guards needed fixing.") + + +if __name__ == "__main__": + main() diff --git a/split_square_verify.cpp b/split_square_verify.cpp index 9c115c4..ad3d2bd 100644 --- a/split_square_verify.cpp +++ b/split_square_verify.cpp @@ -32,24 +32,20 @@ PHLEX_REGISTER_ALGORITHMS(m, config) auto const parent_layer = config.get("parent_layer"); auto const child_layer = config.get("child_layer"); - m.unfold( - "split", - &VectorSplitter::predicate, - &VectorSplitter::unfold, - child_layer, - concurrency::unlimited) - .input_family(product_selector{ - .creator = "input", .layer = parent_layer, .suffix = "numbers"}) + m.unfold("split", + &VectorSplitter::predicate, + &VectorSplitter::unfold, + child_layer, + concurrency::unlimited) + .input_family(product_selector{.creator = "input", .layer = parent_layer, .suffix = "numbers"}) .output_product_suffixes("number"); m.transform( "square", [](int n) { return n * n; }, concurrency::unlimited) - .input_family( - product_selector{.creator = "split", .layer = child_layer, .suffix = "number"}) + .input_family(product_selector{.creator = "split", .layer = child_layer, .suffix = "number"}) .output_product_suffixes("squared"); m.observe( "verify", [](int sq) { assert(sq >= 0); }, concurrency::unlimited) - .input_family( - product_selector{.creator = "square", .layer = child_layer, .suffix = "squared"}); + .input_family(product_selector{.creator = "square", .layer = child_layer, .suffix = "squared"}); } diff --git a/subtract.py b/subtract.py index 5e16c6b..8875690 100644 --- a/subtract.py +++ b/subtract.py @@ -4,6 +4,7 @@ real. It serves as a "Hello, World" equivalent for running Python code. """ + def subtract(i: int, j: int) -> int: """Subtract 'j' from 'i' and return the difference. @@ -36,6 +37,6 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): Returns: None """ - m.transform(subtract, - input_family = config["input"], - output_product_suffixes = config["output"]) + m.transform( + subtract, input_family=config["input"], output_product_suffixes=config["output"] + )