From f0fc76b71d47bd0a649380508187a1aefdbef5a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 21 Aug 2025 17:49:38 +0000 Subject: [PATCH 1/8] Initial plan From 5d9033c88d883efef1fb6fa5eb88681c96b65c38 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 21 Aug 2025 17:58:03 +0000 Subject: [PATCH 2/8] Create comprehensive .github/copilot-instructions.md with validated commands and timing warnings Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/copilot-instructions.md | 235 ++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..c8ce98e121 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,235 @@ +# EICrecon Development Instructions + +EICrecon is a JANA-based reconstruction software for the ePIC detector. This is a complex high-energy physics reconstruction software requiring specialized dependencies and extensive build times. + +**ALWAYS follow these instructions first and fallback to additional search and context gathering only if the information in these instructions is incomplete or found to be in error.** + +## Working Effectively with EICrecon + +### Essential Setup: Use eic-shell Environment + +**CRITICAL: This is the ONLY recommended approach for development. Manual dependency installation is complex and strongly discouraged.** + +**Bootstrap the eic-shell environment:** +```bash +mkdir ~/eic +cd ~/eic +curl --location https://get.epic-eic.org | bash +./eic-shell +``` + +**Alternative if /cvmfs is available:** +```bash +# Note: On JLab ifarm, run 'module load singularity/3.9.5' first +singularity exec /cvmfs/singularity.opensciencegrid.org/eicweb/eic_xl:nightly eic-shell +``` + +**Setup geometry and clone EICrecon:** +```bash +source /opt/detector/epic-main/bin/thisepic.sh +git clone https://github.com/eic/EICrecon +cd EICrecon +``` + +### Build Process + +**Configure and build (NEVER CANCEL - Build takes 30-60 minutes):** +```bash +cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install +cmake --build build --target install -- -j8 +``` + +**CRITICAL TIMING WARNING: Set timeout to 90+ minutes. Build can take 30-60 minutes even with ccache. NEVER CANCEL long-running builds.** + +**Alternative debug build:** +```bash +cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install -DCMAKE_BUILD_TYPE=Debug +cmake --build build --target install -- -j8 +``` + +**Setup environment to use the build:** +```bash +source install/bin/eicrecon-this.sh +``` + +### Testing + +**Run unit tests (NEVER CANCEL - Tests take 15-30 minutes):** +```bash +export LD_LIBRARY_PATH=$PWD/install/lib:$LD_LIBRARY_PATH +export JANA_PLUGIN_PATH=$PWD/install/lib/EICrecon/plugins${JANA_PLUGIN_PATH:+:${JANA_PLUGIN_PATH}} +ctest --test-dir build -V +``` + +**Test basic functionality:** +```bash +eicrecon --help +``` + +Expected output shows usage information with options like `-h`, `-v`, `-c`, `-Pkey=value`, etc. + +**Test plugin loading (basic validation):** +```bash +jana -PPLUGINS=podio,dd4hep +``` + +### Manual Validation Scenarios + +**ALWAYS perform these validation steps after making changes:** + +1. **Basic executable test:** + ```bash + eicrecon --version + eicrecon --configs + ``` + +2. **Plugin validation:** + ```bash + eicrecon -L # List factories + ``` + +3. **Environment validation:** + ```bash + echo $JANA_PLUGIN_PATH + echo $LD_LIBRARY_PATH + ldd install/lib/EICrecon/plugins/*.so | grep -v "not found" || echo "Missing dependencies detected" + ``` + +### Build Configurations and Sanitizers + +**Available build options (use in cmake configure step):** +- `-DCMAKE_BUILD_TYPE=Release` (default, fastest) +- `-DCMAKE_BUILD_TYPE=Debug` (for debugging) +- `-DUSE_ASAN=ON` (Address Sanitizer) +- `-DUSE_TSAN=ON` (Thread Sanitizer - cannot combine with ASAN) +- `-DUSE_UBSAN=ON` (Undefined Behavior Sanitizer) + +**Example with sanitizers:** +```bash +cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install -DCMAKE_BUILD_TYPE=Debug -DUSE_ASAN=ON -DUSE_UBSAN=ON +``` + +### Development Workflow + +**After making code changes:** +```bash +# Rebuild (incremental build is faster) +cmake --build build --target install -- -j8 + +# Run relevant tests +ctest --test-dir build -V -R "specific_test_name" + +# Test functionality +source install/bin/eicrecon-this.sh +eicrecon --help +``` + +**Before committing changes - ALWAYS run:** +```bash +# These must pass or CI will fail +cmake --build build --target install -- -j8 +ctest --test-dir build -V +``` + +## Repository Structure and Navigation + +### Key Directories +- `src/algorithms/` - Core physics algorithms +- `src/factories/` - JANA factory implementations +- `src/services/` - Service components +- `src/tests/` - Unit and integration tests +- `src/benchmarks/` - Performance benchmarks +- `docs/` - Documentation and tutorials +- `cmake/` - CMake configuration files + +### Important Test Suites +- `src/tests/algorithms_test/` - Algorithm unit tests (uses Catch2) +- `src/tests/omnifactory_test/` - Factory framework tests +- `src/tests/tracking_test/` - Tracking algorithm tests +- `src/tests/geometry_navigation_test/` - Geometry tests + +### Commonly Modified Files +When making physics algorithm changes: +1. Check `src/algorithms/` for the relevant algorithm +2. Look for corresponding factory in `src/factories/` +3. Check for tests in `src/tests/algorithms_test/` +4. Update documentation in `docs/` if needed + +## Manual Build (NOT RECOMMENDED) + +**WARNING: Manual dependency installation is extremely complex and time-consuming. Only attempt if eic-shell is absolutely unavailable.** + +Manual build requires installing these dependencies in order: +- C++20 compiler (GCC 10+ or Clang 10+) +- CMake 3.24+ +- Python 3.8+ with pyyaml, jinja2 +- boost 1.70+ +- ROOT 6.26+ (with C++17) +- JANA 2.2.0+ +- fmt 9.0.0+ +- spdlog 1.11.0+ +- PODIO 0.17+ +- EDM4hep 0.7.1+ +- DD4hep 1.21+ +- ACTS 33.0.0+ +- Eigen 3.3+ + +**Each dependency build can take 15-60 minutes. Total time: 4-8 hours.** + +See `docs/get-started/manual-build.md` for complete manual build instructions, but expect significant complexity and troubleshooting. + +## Timing Expectations and Critical Warnings + +**NEVER CANCEL these operations - they are expected to take significant time:** + +| Operation | Expected Time | Minimum Timeout | +|-----------|---------------|-----------------| +| Initial build | 30-60 minutes | 90 minutes | +| Incremental build | 5-15 minutes | 30 minutes | +| Full test suite | 15-30 minutes | 45 minutes | +| Individual test | 1-5 minutes | 10 minutes | +| Manual dependency build | 4-8 hours | Not recommended | + +**Use appropriate timeouts for all long-running commands. The CI system uses ccache and parallel builds which can still take significant time.** + +## Common Issues and Solutions + +**Build fails with missing dependencies:** +- Ensure you're in eic-shell environment +- Run `source /opt/detector/epic-main/bin/thisepic.sh` +- Check CMake configuration output for specific missing packages + +**Tests fail:** +- Verify environment setup: `source install/bin/eicrecon-this.sh` +- Check library paths are correct +- Run individual failing tests for detailed output + +**Performance issues:** +- Use `-j8` for parallel builds (adjust number based on available cores) +- Consider using ccache for repeated builds +- Debug builds are significantly slower than Release builds + +## CI/CD Integration + +The project uses GitHub Actions with the following key checks: +- Builds on multiple compilers (GCC, Clang) +- Multiple build types (Release, Debug) +- Address, Thread, and UB sanitizers +- clang-tidy and include-what-you-use checks +- Extensive physics simulation and reconstruction tests + +**All commits must pass these checks. Run local builds and tests before pushing.** + +## Data Files and Physics Validation + +**Physics reconstruction requires specific input data formats:** +- Input: `.edm4hep.root` files (simulated physics events) +- Output: `.edm4eic.root` files (reconstructed physics data) + +**Sample reconstruction command:** +```bash +export DETECTOR_CONFIG=${DETECTOR}_craterlake +eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root +``` + +**For physics validation, you need appropriate simulation files. The CI system generates these automatically.** \ No newline at end of file From 0e42868cec399f1cc5e5e1a5bbd14868c2903bc7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 21 Aug 2025 18:01:03 +0000 Subject: [PATCH 3/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .github/copilot-instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c8ce98e121..5785f96cff 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -135,7 +135,7 @@ ctest --test-dir build -V ### Key Directories - `src/algorithms/` - Core physics algorithms -- `src/factories/` - JANA factory implementations +- `src/factories/` - JANA factory implementations - `src/services/` - Service components - `src/tests/` - Unit and integration tests - `src/benchmarks/` - Performance benchmarks @@ -232,4 +232,4 @@ export DETECTOR_CONFIG=${DETECTOR}_craterlake eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root ``` -**For physics validation, you need appropriate simulation files. The CI system generates these automatically.** \ No newline at end of file +**For physics validation, you need appropriate simulation files. The CI system generates these automatically.** From 77138f5237fccf88e2775ead97ed575f247eb00a Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Thu, 21 Aug 2025 13:09:49 -0500 Subject: [PATCH 4/8] Update copilot-instructions with corrections and additions --- .github/copilot-instructions.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5785f96cff..294debdc4b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -20,10 +20,22 @@ curl --location https://get.epic-eic.org | bash **Alternative if /cvmfs is available:** ```bash -# Note: On JLab ifarm, run 'module load singularity/3.9.5' first singularity exec /cvmfs/singularity.opensciencegrid.org/eicweb/eic_xl:nightly eic-shell ``` +This approach is achieved by the following GitHub Actions snippet (inside : +```yaml + steps: + - name: Ensure CernVM-FS is available + uses: cvmfs-contrib/github-action-cvmfs@v5 + - name: Run inside the eic_xl:nightly environment + uses: eic/run-cvmfs-osg-eic-shell@main + with: + platform-release: "eic_xl:nightly" + run: | + echo +``` + **Setup geometry and clone EICrecon:** ```bash source /opt/detector/epic-main/bin/thisepic.sh @@ -70,7 +82,7 @@ Expected output shows usage information with options like `-h`, `-v`, `-c`, `-Pk **Test plugin loading (basic validation):** ```bash -jana -PPLUGINS=podio,dd4hep +eicrecon -Pplugins=podio,dd4hep ``` ### Manual Validation Scenarios From 9ca579b1300eb049f917eefee76208df0145e7e5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 21 Aug 2025 18:14:52 +0000 Subject: [PATCH 5/8] Fix incomplete text in GitHub Actions snippet description Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 294debdc4b..de0324bfb5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -23,7 +23,7 @@ curl --location https://get.epic-eic.org | bash singularity exec /cvmfs/singularity.opensciencegrid.org/eicweb/eic_xl:nightly eic-shell ``` -This approach is achieved by the following GitHub Actions snippet (inside : +This approach is achieved by the following GitHub Actions snippet (inside a workflow): ```yaml steps: - name: Ensure CernVM-FS is available From 88897ccc6c7706dabae5e1304491176991719763 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 15:10:40 +0000 Subject: [PATCH 6/8] Add header alphabetization requirements for IWYU compliance Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/copilot-instructions.md | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index de0324bfb5..6ba2067b93 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -245,3 +245,44 @@ eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root ``` **For physics validation, you need appropriate simulation files. The CI system generates these automatically.** + +## Code Style and Header Organization + +### Header Include Order and Alphabetization + +**CRITICAL: Headers must be sorted alphabetically within each blank-line delimited header block to pass include-what-you-use (IWYU) checks.** + +**Header organization rules:** +1. **Always sort headers alphabetically within each group** separated by blank lines +2. **Group headers in this order:** + - System/standard library headers (e.g., ``, ``, ``, ``) + - Third-party library headers (e.g., ``, ``, ``, ``) + - Project-specific headers (e.g., `"algorithms/..."`, `"services/..."`, `"utilities/..."`) + +**Example correct header ordering:** +```cpp +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "algorithms/calorimetry/CalorimeterHitDigi.h" +#include "services/geometry/dd4hep/DD4hep_service.h" +#include "services/log/Log_service.h" +``` + +**IWYU enforcement:** The CI system runs include-what-you-use with `--reorder` and `fix_includes.py --reorder` to automatically detect and fix header ordering violations. If headers are not properly sorted: +- CI will fail with IWYU violations +- An automatic PR may be created with header fixes +- Manual fixes must sort headers alphabetically within each group + +**When editing existing code:** +- Always maintain alphabetical ordering when adding new headers +- Check that new headers are placed in the correct group (system, third-party, or project) +- Respect existing blank line separations between header groups +- Use the IWYU mapping file (`.github/iwyu.imp`) rules for header substitutions From a39768cd17b730374f96726538670d5ddcb87c38 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 17:45:11 +0000 Subject: [PATCH 7/8] Add explicit instructions for header include syntax (quotes vs angle brackets) Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/copilot-instructions.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6ba2067b93..56bc38f4ec 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -254,7 +254,10 @@ eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root **Header organization rules:** 1. **Always sort headers alphabetically within each group** separated by blank lines -2. **Group headers in this order:** +2. **Use correct include syntax:** + - **Angle brackets `<>`** for system/standard library headers and external dependencies + - **Quotes `""`** for project-specific/local headers +3. **Group headers in this order:** - System/standard library headers (e.g., ``, ``, ``, ``) - Third-party library headers (e.g., ``, ``, ``, ``) - Project-specific headers (e.g., `"algorithms/..."`, `"services/..."`, `"utilities/..."`) @@ -283,6 +286,7 @@ eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root **When editing existing code:** - Always maintain alphabetical ordering when adding new headers +- **Use angle brackets `<>` for system and third-party headers, quotes `""` for project headers** - Check that new headers are placed in the correct group (system, third-party, or project) - Respect existing blank line separations between header groups - Use the IWYU mapping file (`.github/iwyu.imp`) rules for header substitutions From fdf02d0124ba2198ad209882a94ad1d2be6c49f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 17:51:50 +0000 Subject: [PATCH 8/8] Add conventional commits requirements for breaking change identification Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/copilot-instructions.md | 89 +++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 56bc38f4ec..dd5921fd37 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -290,3 +290,92 @@ eicrecon -Ppodio:output_file=output.edm4eic.root input_simulation.edm4hep.root - Check that new headers are placed in the correct group (system, third-party, or project) - Respect existing blank line separations between header groups - Use the IWYU mapping file (`.github/iwyu.imp`) rules for header substitutions + +## Conventional Commits and Breaking Changes + +### Commit Message Format + +**CRITICAL: All pull requests must follow the [Conventional Commits specification](https://www.conventionalcommits.org/en/v1.0.0/) to clearly highlight breaking changes.** + +**Standard commit message format:** +``` +[optional scope]: + +[optional body] + +[optional footer(s)] +``` + +**Common commit types:** +- `feat:` - New features +- `fix:` - Bug fixes +- `docs:` - Documentation changes +- `style:` - Code style changes (formatting, etc.) +- `refactor:` - Code refactoring without functional changes +- `test:` - Adding or modifying tests +- `chore:` - Maintenance tasks, build changes +- `perf:` - Performance improvements + +**Examples of conventional commit messages:** +``` +feat(tracking): add silicon tracker hit reconstruction +fix(calorimetry): correct energy calibration for ECAL clusters +docs(readme): update build instructions for new dependencies +refactor(algorithms): simplify cluster formation algorithm +test(tracking): add unit tests for track fitting +``` + +### Breaking Changes in EICrecon Context + +**CRITICAL: Use `BREAKING CHANGE:` footer or `!` suffix for any changes that affect user workflows.** + +**Consider as breaking changes:** + +1. **Command-line interface changes:** + - Removal or renaming of `eicrecon` command-line options + - Changes to argument parsing that affect existing scripts + - Modifications to configuration parameter names or behavior + - Changes to plugin loading syntax or requirements + +2. **Output collection changes:** + - Renaming of output collection names (e.g., `EcalBarrelClusters` → `ECALClusters`) + - Removal of existing output collections that users depend on + - Significant changes to collection content structure or data members + - Changes to default output file naming conventions + +**Examples of breaking change commits:** +``` +feat!: change ECAL cluster collection name for consistency + +BREAKING CHANGE: EcalBarrelClusters collection renamed to ECALBarrelClusters +to match naming convention. Update analysis scripts to use new name. + +feat(cli): add new reconstruction mode option + +BREAKING CHANGE: --mode argument now required for eicrecon execution. +Add --mode=default to existing scripts to maintain current behavior. + +refactor(algorithms): restructure tracking output collections + +BREAKING CHANGE: TrackCandidates collection split into ReconstructedTracks +and TrackParameters. Analysis code must be updated to use new collections. +``` + +**Non-breaking changes (safe to implement without `BREAKING CHANGE`):** +- Algorithm improvements that produce better but compatible results +- Addition of new optional command-line arguments +- Addition of new output collections alongside existing ones +- Performance optimizations that don't change interfaces +- Internal refactoring that doesn't affect user-facing APIs + +**Validation before marking as breaking:** +```bash +# Test command-line compatibility +eicrecon --help # Verify existing options still work +eicrecon -Pkey=value input.edm4hep.root # Test parameter syntax + +# Test output collection compatibility +# Run reconstruction and verify expected collections exist +eicrecon -Ppodio:output_file=test.root input.edm4hep.root +# Check collection names match expectations +```