Skip to content

Fdc strip peakamp rtj - #422

Open
rjones30 wants to merge 4 commits into
masterfrom
fdc_strip_peakamp_rtj
Open

Fdc strip peakamp rtj#422
rjones30 wants to merge 4 commits into
masterfrom
fdc_strip_peakamp_rtj

Conversation

@rjones30

@rjones30 rjones30 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@gluex

gluex commented Nov 3, 2026

Copy link
Copy Markdown

Build status for this pull request: FAILURE

Build log: make_fdc_strip_peakamp_rtj.log
Build report: report_fdc_strip_peakamp_rtj.txt

@rjones30
rjones30 requested a review from nsjarvis November 3, 2026 03:18
@aaust

aaust commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

I assume it fails the test because of the nightly-build. I'll restart it tomorrow

@rjones30 rjones30 mentioned this pull request Nov 3, 2026
Merged
@fjb994286-sys

fjb994286-sys commented Nov 3, 2026 via email

Copy link
Copy Markdown

@gluex

gluex commented Nov 3, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_fdc_strip_peakamp_rtj.log
Build report: report_fdc_strip_peakamp_rtj.txt

nsjarvis
nsjarvis requested changes Nov 4, 2026
View reviewed changes

@nsjarvis nsjarvis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new merge code with the peak amp in it looks good.

I don't like what you have done in the smearer with the peak amp and a-scale.
What I think you should do to create the peak amp is this:
Set it equal to q.
Make a separate raw_amp = q/a_scale
Compare raw_amp against the threshold and discard the hit it if is too small.

This will model the effect of changing pressure using the gain factors that have already been provided in a_scale by calibrating the later runs. I can easily calibrate the earlier run periods similarly. We do not need a different a_scale for simulations.

In the CDC smearer we even have code to model the saturation of raw_amp, that might not be necessary for the FDC as its readout has 3 more bits for pulse height than the CDC's readout does.

@rjones30

rjones30 commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thanks, Naomi. I agree with modelling the gain through the per-run a_scale, and with cutting on raw_amp = q/a_scale. I would like to keep peakAmp = q/a_scale, though, and I'll explain why below.

a_scale already follows the gain. /FDC/digi_scales FDC_ADC_ASCALE has per-run values for 2019-11, entered on 2022-03-22:

  • 0.179-0.191 for runs 71353-71362 (10-11 January 2020);
  • 0.307-0.325 for runs 71483-71598 (19-27 January 2020).

The ratio is 0.60. That is exactly the drop we measure in the FDC strip signal between those runs, which is behind the apparent beam-current dependence of our 2019-11 rho(0) cross section. Run 71350 and the September 2020 runs 73223-73227 have no entry and use the default 0.2. From the measured signal, the September runs should be near 0.36. It would be great if you could calibrate those.

Why peakAmp should be q/a_scale. For MC, DEventSourceHDDM takes the fdcDigihit peakAmp as DFDCHit::pulse_height. In data, DFDCHit_factory computes q = a_scale × pulse_height, so the data pulse_height is q/a_scale. DFDCPseudo_factory uses the charge built from pulse_height in absolute terms, in three places:

  • the FDC:CHARGE_THRESHOLD cut;
  • the assumed strip resolution, FDC_RES_PAR1/q + FDC_RES_PAR2;
  • dE_amp.

So the MC pulse height has to be on the data scale. These are median pseudopoint charges (DFDCPseudo q) for pion tracks within 0.2 cm of the wire, in the first January 2020 runs:

median pseudopoint q
data 5.85
MC, peakAmp = q/a_scale 4.69
MC, peakAmp = q (estimated) about 0.9

With peakAmp = q/a_scale the MC pseudopoint charge also follows the January step: relative to the first runs it is 0.62 in MC and 0.60 in data. With peakAmp = q it would stay flat.

Proposal:

  • Keep peakAmp = q/a_scale.
  • Add the threshold you suggest: drop the strip hit if raw_amp = q/a_scale is below the readout threshold in ADC counts. Which value should we use? Is it in CCDB, or should it be taken from the fa125 configuration?
  • Calibrate a_scale for the runs that still have the default.

If you agree, I'll update the PR accordingly.

@nsjarvis

nsjarvis commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

The MC data is supposed to mimic calibrated data, so its peakAmp should be constant with time.

I have no idea what happened to the 2019-11 calibrations. We can see in the Demon plot that dE/dx is not flat. I have new values in /work/halld/njarvis/gluex20/gains/rest1/newgains - I think these should not go into the main ccdb yet, maybe they need a new variation. TBD. No problem using them in sqlite for tests.

We should take the thresholds from the experimental data, they can be extracted from the BOR records. This will take a while. Is there anywhere in ccdb to put them? There should be one threshold per channel, I suppose we could cheat by using one threshold for all.

@sdobbs

sdobbs commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

there's not currently a table for per-channel thresholds, but adding one is easy enough

@rjones30

rjones30 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

"The MC data is supposed to mimic calibrated data, so its peakAmp should be constant with time."

Should the MC response be run-independent and the corrections always applied to real data when running the reconstruction? In principle, but in practice that can get prohibitively expensive. That would mean that we have to rerun all of the reconstruction on 2020 data before we can get a cross section with forward charged tracks that we can believe. Is standing by that principle worth the cost? Or can we move into mcsmear some of the corrections that one would prefer to have had already when the data were reconstructed? The reconstruction is what it is, and the job of the simulation (mcsmear) from this time forward is to produce sims that look like the data did ("calibrated" is time-dependent) at the time the reconstruction was run. I would agree with your statement if it were applied to the output from hdgeant4. Applied to the output from mcsmear, I would question the wisdom of it.

@nsjarvis

nsjarvis commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

We do plan to re-reconstruct most of the older run periods with the correct geometry anyway.

@nsjarvis

nsjarvis commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Actually, I don't think it's necessary to redo the 2019-11 reconstruction just because we changed the FDC gain calibration. As nobody cuts on FDC dE/dx, the only thing that uses it is the PIDFOM. If it was necessary to include new gains that could be done with a scaling factor per run in the analysis trees.

While we are improving the FDC smearing, we should do it properly.
Set peak-amp equal to q.
Make a separate raw_amp = q/a_scale.
Compare raw_amp against the threshold and discard the hit it if is too small.

Using my new calibrations, this should remove simulated hits when the gas gain is too low. Apart from losing hits at small dE/dx, the sim dE/dx should be approximately unchanging throughout the run period, which will match the real properly calibrated dE/dx (although nobody is using this anyway).

@rjones30

rjones30 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

For this gain and threshold, it is not the PID that is relevant, it is the tracking itself. We get the tracking efficiency wrong in MC without having this run-dependent inefficiency right.

rjones30 added a commit that referenced this pull request Nov 9, 2026
@rjones30 @claude
mcsmear: FDC gas-gain, signal attenuation and threshold options [rtj]
4a42d96 
New command-line options for the FDC smearing. Every default leaves the
output unchanged.

  FDC:GAIN_SCALE        gas-gain factor on the strip charge q and on the
                        anode wire dE, applied before the thresholds (1)
  FDC:GAIN_PKG1..4      extra gain factor per FDC package (modules 1-2,
                        3-4, 5-6, 7-8), times FDC:GAIN_SCALE (1)
  FDC:GAIN_SIGMA s      avalanche gain fluctuation: one log-normal factor
                        exp(s G - s^2/2) per chamber and track, shared by
                        the wire hit and the strips it induces (0)
  FDC:DOCA_ATTEN a      wire dE and the track's strip q in the chamber
                        scaled by max(0.05, 1 - a doca^2), doca in cm (0)
  FDC:STRIP_THRESHOLD   strip charge threshold in pC; < 0 keeps
                        FDC_THRESHOLD_FACTOR * FDC_PED_NOISE from CCDB (-1)
  FDC:WIRE_THRESHOLD_KEV  anode threshold on the gain-scaled wire dE in keV;
                        < 0 keeps FDC_THRESH_KEV from CCDB (-1)

Motivation: the FDC gas gain of the real detector changes with time and
differs between packages (e.g. a 40% drop of the strip signal between
2019-11 runs 71362 and 71483), which changes the FDC hit efficiency and
with it the tracking efficiency. With per-period gains fitted only to the
data's FDC plane-used fractions (not to any cross section), the rho(0)
photoproduction cross section of ten run segments (2018-01 to 2023-01)
agrees to about 2% (RMS, beyond statistics).

Builds on the FDC options and peakAmp of PR #422; independent of #423.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rjones30

rjones30 commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I pushed two commits to this PR. The new options all default to off, and with the defaults the output is byte-identical to before.

  • bd0cb53 FDC:RAW_AMP_THRESHOLD (ADC counts): a strip hit is dropped if raw_amp = q / a_scale is below the threshold, as you suggested, Naomi. Until we have per-channel thresholds it is one value for all strips.
  • 4a42d96 FDC gas-gain options: FDC:GAIN_SCALE, FDC:GAIN_PKG1..4 (per package), FDC:GAIN_SIGMA, FDC:DOCA_ATTEN, FDC:STRIP_THRESHOLD, FDC:WIRE_THRESHOLD_KEV.

I kept peakAmp = q / a_scale. The reconstruction reads the MC fdcDigihit peakAmp as DFDCHit::pulse_height, and q as DFDCHit::q, and it uses the two for different things:

  • pulse_height for the pseudopoint charge threshold, the strip resolution and dE_amp;
  • q for the pseudopoint dE.

In data, q = a_scale × pulse_height. Carrying both lets the MC reproduce that relation without any change to halld_recon.

With these options, ten run segments from 2018-01 to 2023-01 give a rho0 cross section that agrees to about 2% (RMS beyond statistics). The gains were fitted only to the FDC plane-used fractions of the data, not to the cross section. These are the command-line values used:

segment GAIN_SCALE PKG1 PKG2 PKG3 PKG4
2018-01 150nA 1.753 0.981 1.160 1.056 0.832
2018-08 500nA 1.527 1.023 1.074 1.113 0.818
2018-08 180nA 1.645 1.024 1.130 1.092 0.791
2019-11 150nA 1.539 1.078 1.132 1.055 0.777
2019-11 250nA 1.213 1.133 1.073 1.025 0.802
2019-11 350nA early 1.177 1.147 1.054 1.030 0.803
2019-11 350nA late 1.033 1.044 1.105 1.105 0.785
2023-01 early 1.091 1.089 1.073 1.061 0.807
2023-01 middle 1.157 1.090 1.052 1.080 0.808
2023-01 late 1.116 1.036 1.096 1.076 0.819

All segments: FDC:DOCA_ATTEN=3.5 FDC:GAIN_SIGMA=0.7 FDC:STRIP_THRESHOLD=16 FDC:WIRE_THRESHOLD_KEV=0.15 (raw_amp threshold off).

Run-dependent constants belong in CCDB rather than on the command line. I am open to a friendly amendment that puts them there before this PR is merged.

@gluex

gluex commented Nov 9, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_fdc_strip_peakamp_rtj.log
Build report: report_fdc_strip_peakamp_rtj.txt

@nsjarvis

nsjarvis commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Please could you move the first commit into a separate PR?

@nsjarvis

nsjarvis commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

The values for 2019-11 in CCDB's /FDC/digi_scales were entered after the REST1 production finished. At the time of the production, there was one default value of 0.2 .

@nsjarvis

nsjarvis commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Also, please could you move this commit into a separate PR and add some code which will try to read the cathode strip thresholds from CCDB tables FDC/packageN/strip_thresholds and default to 50 if the table does not exist?

@rjones30

rjones30 commented Nov 9, 2026 via email

Copy link
Copy Markdown
Contributor Author

@nsjarvis

nsjarvis commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ah sorry, I tried to paste in links to the commits but I guess that failed.

Would it be possible to make separate PRs for these two, please? :

c362337 - random hit merging and the addition of peak_amp to the output.
bd0cb53 - cathode strip thresholds.

@rjones30

rjones30 commented Nov 9, 2026 via email

Copy link
Copy Markdown
Contributor Author

rjones30 and others added 3 commits November 9, 2026 14:56
@rjones30 @claude
mcsmear: new FDC smearing options FDC:DRIFT_SMEAR_NS, FDC:CATHODE_SIG…
db67796 
…MA and

FDC:EFFVSDOCA_PAR0..3; defaults leave the output unchanged [rtj]

 - FDC:DRIFT_SMEAR_NS (ns, default 0 = off): a Gaussian drift-time shift
   drawn once per track per chamber and applied to the wire hit and to the
   strip hits of that track together. The strips see the charge induced by
   the wire avalanche, so a fluctuation of the drift delay moves both by
   the same amount; the existing independent smearing (FDC_TDRIFT_SIGMA)
   is kept. No random numbers are drawn when the option is 0.
 - FDC:CATHODE_SIGMA (microns, default < 0 = CCDB): overrides
   FDC_CATHODE_SIGMA from /FDC/fdc_parms, which sets the strip pedestal
   noise, so the strip resolution can be tuned without a CCDB change.
 - FDC:EFFVSDOCA_PAR0..3: the parameters of the doca-dependent wire
   efficiency, previously hard-coded; the defaults are the former values.

The settings in use are printed at startup.

In the GlueX rho0 data/MC study of 2026-10/11 the tuned values were
FDC:DRIFT_SMEAR_NS=3, FDC:CATHODE_SIGMA=85 and FDC:USE_EFFVSDOCA=0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 52c79ad)
@rjones30 @claude
mcsmear: apply the FDC anode discriminator threshold FDC_THRESH_KEV t…
10dd7e4 
…o the

wire hits [rtj]

FDC_THRESH_KEV was read from /FDC/fdc_parms but never used by mcsmear, so
the only wire threshold in the simulation chain was the one applied by
hdgeant4 to each track segment. mcsmear now drops an anode truth hit whose
deposited energy is below FDC_THRESH_KEV. The cut acts on the energy that
hdgeant4 summed over the segments it combined into the hit, which is closer
to what the discriminator sees, and it makes the effective wire threshold
adjustable without rerunning hdgeant4 (by giving mcsmear a CCDB in which
FDC_THRESH_KEV differs from the one used for hdgeant4). The value in use is
printed at startup. No random numbers are drawn by the cut.

With the same CCDB for both programs the output is unchanged: every hit
that hdgeant4 kept already has dE above the threshold. (In the drift-cluster
mode of hdgeant4, DRIFTCLUSTERS != 0, dE holds the integrated pulse instead
of a deposited energy, so FDC_THRESH_KEV should be set accordingly there.)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 68b8a9b)
@rjones30 @claude
mcsmear: FDC gas-gain, signal attenuation and threshold options [rtj]
782fb13 
New command-line options for the FDC smearing. Every default leaves the
output unchanged.

  FDC:GAIN_SCALE        gas-gain factor on the strip charge q and on the
                        anode wire dE, applied before the thresholds (1)
  FDC:GAIN_PKG1..4      extra gain factor per FDC package (modules 1-2,
                        3-4, 5-6, 7-8), times FDC:GAIN_SCALE (1)
  FDC:GAIN_SIGMA s      avalanche gain fluctuation: one log-normal factor
                        exp(s G - s^2/2) per chamber and track, shared by
                        the wire hit and the strips it induces (0)
  FDC:DOCA_ATTEN a      wire dE and the track's strip q in the chamber
                        scaled by max(0.05, 1 - a doca^2), doca in cm (0)
  FDC:STRIP_THRESHOLD   strip charge threshold in pC; < 0 keeps
                        FDC_THRESHOLD_FACTOR * FDC_PED_NOISE from CCDB (-1)
  FDC:WIRE_THRESHOLD_KEV  anode threshold on the gain-scaled wire dE in keV;
                        < 0 keeps FDC_THRESH_KEV from CCDB (-1)

Motivation: the FDC gas gain of the real detector changes with time and
differs between packages (e.g. a 40% drop of the strip signal between
2019-11 runs 71362 and 71483), which changes the FDC hit efficiency and
with it the tracking efficiency. With per-period gains fitted only to the
data's FDC plane-used fractions (not to any cross section), the rho(0)
photoproduction cross section of ten run segments (2018-01 to 2023-01)
agrees to about 2% (RMS, beyond statistics).

Builds on the FDC smearing options of this PR. Independent of the strip
peakAmp PR (the thresholds act on the charge q, not on peakAmp) and of #423.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 4a42d96)
@rjones30
rjones30 force-pushed the fdc_strip_peakamp_rtj branch from bd0cb53 to 782fb13 Compare November 9, 2026 20:33
This was referenced Nov 9, 2026
Open
Open
@rjones30

rjones30 commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Naomi, done as you asked:

This PR now holds the other three commits: the FDC smearing options, the anode threshold FDC_THRESH_KEV in mcsmear, and the gas-gain options. They apply to master without #426 or #427. PR #426
stands alone; #427 needs #426.

With #426 + this PR + #427 + #423 merged and -PFDC:RAW_AMP_THRESHOLD=0 added to our options, the output of our rho0 study's mcsmear is reproduced byte for byte (tested on 300 events of a 2018-01 chunk). The default of 50 matches what our gain fit implies: in the segments I checked where a_scale is calibrated, our fitted 16 pC strip threshold corresponds to 46-51 counts.

@gluex

gluex commented Nov 9, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_fdc_strip_peakamp_rtj.log
Build report: report_fdc_strip_peakamp_rtj.txt

@nsjarvis

nsjarvis commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Thanks, this should make it easier to test all the changes.

aaust
aaust reviewed Nov 11, 2026
View reviewed changes
Comment thread src/programs/Simulation/mcsmear/FDCSmearer.cc Resolved
@rjones30

rjones30 commented Nov 11, 2026 via email

Copy link
Copy Markdown
Contributor Author

@aaust

aaust commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Thanks, I suggest to turn off the old parametrization of the efficiency by default, or simply drop the whole part of the code.

@rjones30

rjones30 commented Nov 12, 2026 via email

Copy link
Copy Markdown
Contributor Author

@aaust

aaust commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Afaik, the parameters are only hard-coded in FDCSmearer.cc, they are not read in from CCDB.

@rjones30 @claude
mcsmear: remove the old FDC wire efficiency-vs-doca curve (GetEfficie…
a9b8c02 
…ncyVsDOCA, FDC:USE_EFFVSDOCA, FDC:EFFVSDOCA_PAR0..3), superseded by FDC:DOCA_ATTEN [rtj]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rjones30

rjones30 commented Nov 12, 2026 via email

Copy link
Copy Markdown
Contributor Author

@gluex

gluex commented Nov 12, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_fdc_strip_peakamp_rtj.log
Build report: report_fdc_strip_peakamp_rtj.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaust aaust aaust left review comments
@nsjarvis nsjarvis nsjarvis requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rjones30 @gluex @aaust @fjb994286-sys @nsjarvis @sdobbs