feat: introduce digitization #663

asalzburger · 2021-01-21T17:27:03Z

This PR superseded #652 and introduces the geometric digitization + the possibility to configure it with json.

In detail:

This PR introduces a new configuration based on Json for digitization
It adds a new unit test Examples/JsonDigitizationConfigTests.cpp
It introduces the new DigitizationAlgorithm capable of smearing and geometric digitization
It allows to use the same RootDigitizationWriter for SmearingAlgorithm and DigitizationAlgorithm

When deployed, the HitSmearing and PlanarSteppingAlgorithm can become osbolete, and finally the Plugins/Digitization and Plugins/Identification can vanish.

Here are two example files snippes for the Pixel detector of the TrackML detector

analog digitization (blue)

{
  "acts-geometry-hierarchy-map": {
    "format-version": 0,
    "value-identifier": "digitization-configuration"
  },
  "entries": [
    {
      "layer": 2,
      "value": {
        "geometric": {
          "digital": false,
          "indices": [
            0,
            1
          ],
          "segmentation": {
            "binningdata": [
              {
                "bins": 336,
                "max": 8.399999618530273,
                "min": -8.399999618530273,
                "option": "open",
                "type": "equidistant",
                "value": "binX"
              },
              {
                "bins": 1280,
                "max": 36.0,
                "min": -36.0,
                "option": "open",
                "type": "equidistant",
                "value": "binY"
              }
            ]
          },
          "thickness": 0.15,
          "threshold": 0.01
        }
      },
      "volume": 8
    }]
}

digital digitization (red)

(only showing the change)

          "digital": false,

The results can be seen in the following plot:

codecov · 2021-01-21T18:10:56Z

Codecov Report

Merging #663 (f630875) into master (f61a219) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #663   +/-   ##
=======================================
  Coverage   49.07%   49.07%           
=======================================
  Files         331      331           
  Lines       16568    16568           
  Branches     7722     7722           
=======================================
  Hits         8130     8130           
  Misses       3006     3006           
  Partials     5432     5432

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f61a219...c57f696. Read the comment docs.

…e more work

asalzburger · 2021-01-29T12:03:27Z

Hi @XiaocongAi - sorry for the bit lengthy Pr, but it should be quite clear. There will be a bit more code to come for the covariance assignment and the smearing generation of charge and lorentz vector, but this can be done as an addon.

asalzburger · 2021-01-29T13:54:34Z

@paulgessinger - I tried three times now with the macos build on this PR but it constantly fails :-(

asalzburger · 2021-02-01T10:40:18Z

Ok, CI finally runs through - @xai - this one is ready for review

XiaocongAi

Hi @asalzburger, I have not looked into the Json part in great detail, but in general, the PR looks very nice to me. I have only a few minor comments.

Examples/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationAlgorithm.hpp

Examples/Algorithms/Digitization/src/DigitizationAlgorithm.cpp

Examples/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationConfig.hpp

XiaocongAi · 2021-02-02T15:45:28Z

Examples/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationAlgorithm.hpp

+  struct DigitizedParameters {
+    std::vector<Acts::BoundIndices> indices = {};
+    std::vector<Acts::ActsScalar> values = {};
+    std::vector<Acts::ActsScalar> covariances = {};


I guess this vector is for just the errors of the parameters (not their correlation), right? Personally, I would expect a matrix from the name covariance. Is it better to rename it to e.g. errors or uncertainties?

Measurement directions are by all means independent for our purpose.
We could write 'variances' ?

Sounds good!

Examples/Io/Json/src/JsonDigitizationConfig.cpp

asalzburger · 2021-02-02T18:36:50Z

@XiaocongAi - I think I have addressed everything & simplified the code by removing the 'Void...Generators' by just having void functions.

XiaocongAi

HI @asalzburger , thank you for the changes. I will approve.
One thing to confirm though, it looks to me we don't need the SmearingAlgorithm since it's basically contained in this DigitizationAlgorithm when we use the smearing instead of geometric in the Json input. But you mentioned the HitSmearing. Which one are we going to remove, the SmearingAlgorithm or the HitSmearing?

XiaocongAi · 2021-02-02T18:20:32Z

Examples/Algorithms/Digitization/include/ActsExamples/Digitization/DigitizationConfig.hpp

+/// @return a charge to which the threshold can be applied
+using ChargeGenerator = std::function<Acts::ActsScalar(
+    Acts::ActsScalar, Acts::ActsScalar, RandomEngine&)>;
+/// Takes as an argument the clsuter size and an random engine


Suggested change

/// Takes as an argument the clsuter size and an random engine

/// Takes as an argument the cluster size and an random engine

XiaocongAi · 2021-02-02T18:39:31Z

Tests/UnitTests/Examples/Json/JsonDigitizationConfigTests.cpp

+BOOST_AUTO_TEST_CASE(DigitizationConfigRoundTrip) {
+  std::ofstream out;
+
+  // As all SurfaceBounds have the same streaming API only a one is


Suggested change

// As all SurfaceBounds have the same streaming API only a one is

// As all SurfaceBounds have the same streaming API only one is

asalzburger · 2021-02-02T20:07:00Z

Ideally, both, 'HitSmearing' and 'SmearingAlgorithm' could go, but I would like to keep the latter as we can configure it through command options.

@asalzburger

@asalzburger I think this was a small copy paste error in #663 otherwise the branch would be unreachable.

asalzburger added 2 commits January 21, 2021 18:24

adding DigitizationConfig converter

7780014

Plugin -> Io

5cf55b6

asalzburger self-assigned this Jan 21, 2021

asalzburger added Component - Examples Affects the Examples module Feature Development to integrate a new feature labels Jan 21, 2021

asalzburger added this to the next milestone Jan 21, 2021

asalzburger added the 🚧 WIP Work-in-progress label Jan 21, 2021

asalzburger added 6 commits January 22, 2021 17:39

adding new DigitizationAlgorithm

8558b5c

first full chain of DigitizationAlgorithm

7d0374f

Merge branch 'master' into feat-digitization-example

8fe1d70

first full set of DigitizationAlgorithm, Json converter needs a littl…

e517c17

…e more work

full chain

be210ae

digitization works

168378a

asalzburger requested a review from XiaocongAi January 29, 2021 11:56

asalzburger removed the 🚧 WIP Work-in-progress label Jan 29, 2021

Merge branch 'master' into feat-digitization-example

53b34e4

Merge branch 'master' into feat-digitization-example

b1538b6

Merge branch 'master' into feat-digitization-example

80d72d7

asalzburger mentioned this pull request Jan 29, 2021

feat: update ROOT writers to new digitization #682

Closed

adding cont ref to avoid copying

2658623

asalzburger added 2 commits February 1, 2021 14:28

Merge branch 'master' into feat-digitization-example

ad1727c

Merge branch 'master' into feat-digitization-example

6d29672

XiaocongAi reviewed Feb 2, 2021

View reviewed changes

addressing PR comments

c57f696

XiaocongAi approved these changes Feb 2, 2021

View reviewed changes

asalzburger merged commit bb07ac1 into acts-project:master Feb 2, 2021

asalzburger deleted the feat-digitization-example branch February 2, 2021 20:07

asalzburger mentioned this pull request Feb 5, 2021

Add configurable segmentation and resolution information #138

Closed

paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Feb 9, 2021

feat: introduce digitization (acts-project#663)

1ae6b5a

paulgessinger modified the milestones: next, v6.0.0 Mar 2, 2021

gagnonlg mentioned this pull request Aug 19, 2021

Reestablish the digitization simulation in Fatras #139

Closed

7 tasks

AJPfleger mentioned this pull request Nov 2, 2023

fix: Digitization::Digital in to_json() was skipped #2615

Merged

kodiakhq bot pushed a commit that referenced this pull request Nov 2, 2023

fix: Digitization::Digital in to_json() was skipped (#2615)

314529f

@asalzburger I think this was a small copy paste error in #663 otherwise the branch would be unreachable.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce digitization #663

feat: introduce digitization #663

asalzburger commented Jan 21, 2021 •

edited

Loading

codecov bot commented Jan 21, 2021 •

edited

Loading

asalzburger commented Jan 29, 2021

asalzburger commented Jan 29, 2021

asalzburger commented Feb 1, 2021

XiaocongAi left a comment

XiaocongAi Feb 2, 2021

asalzburger Feb 2, 2021

XiaocongAi Feb 2, 2021

asalzburger commented Feb 2, 2021

XiaocongAi left a comment

XiaocongAi Feb 2, 2021

XiaocongAi Feb 2, 2021

asalzburger commented Feb 2, 2021

	/// Takes as an argument the clsuter size and an random engine
	/// Takes as an argument the cluster size and an random engine

	// As all SurfaceBounds have the same streaming API only a one is
	// As all SurfaceBounds have the same streaming API only one is

feat: introduce digitization #663

feat: introduce digitization #663

Conversation

asalzburger commented Jan 21, 2021 • edited Loading

codecov bot commented Jan 21, 2021 • edited Loading

Codecov Report

asalzburger commented Jan 29, 2021

asalzburger commented Jan 29, 2021

asalzburger commented Feb 1, 2021

XiaocongAi left a comment

Choose a reason for hiding this comment

XiaocongAi Feb 2, 2021

Choose a reason for hiding this comment

asalzburger Feb 2, 2021

Choose a reason for hiding this comment

XiaocongAi Feb 2, 2021

Choose a reason for hiding this comment

asalzburger commented Feb 2, 2021

XiaocongAi left a comment

Choose a reason for hiding this comment

XiaocongAi Feb 2, 2021

Choose a reason for hiding this comment

XiaocongAi Feb 2, 2021

Choose a reason for hiding this comment

asalzburger commented Feb 2, 2021

asalzburger commented Jan 21, 2021 •

edited

Loading

codecov bot commented Jan 21, 2021 •

edited

Loading