Project

General

Profile

Bug #20685

Bug in ISCalcSeparate::EFieldAtStep

Added by Tingjun Yang about 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Category:
-
Target version:
-
Start date:
08/27/2018
Due date:
% Done:

100%

Estimated time:
Occurs In:
Experiment:
-
Co-Assignees:
Duration:

Description

Ajib Paudel found a bug that affects the calculation of the electric field in the simulation when SCE is enabled:
http://nusoft.fnal.gov/larsoft/doxsvn/html/ISCalcSeparate_8cxx_source.html#l00209

  212     if (fSCE->EnableSimEfieldSCE())
  213       {
  214         fEfieldOffsets = fSCE->GetEfieldOffsets(geo::Point_t{x,y,z});
  215         EField = std::sqrt( (efield + efield*fEfieldOffsets.X())*(efield + efield*fEfieldOffsets.X()) +
  216                             (efield*fEfieldOffsets.Y()+efield*fEfieldOffsets.Y()) +
  217                             (efield*fEfieldOffsets.Z()+efield*fEfieldOffsets.Z()) );
  218       }

The last two terms should be:
(efield*fEfieldOffsets.Y()*efield*fEfieldOffsets.Y()) +
(efield*fEfieldOffsets.Z()*efield*fEfieldOffsets.Z())

I think this bug affected MicroBooNE MCC8 and ProtoDUNE MCC10 and should be fixed as soon as possible.

Lynn, please make sure this bug fix is included in this week's release as we are still preparing ProtoDUNE MCC11. Thanks.

History

#1 Updated by Kyle Knoepfel about 1 year ago

  • Status changed from New to Assigned

Mike, can this be ready for this week's release?

#2 Updated by Kyle Knoepfel about 1 year ago

We encourage you to use a library specially designed for these linear-algebra problems (e.g. SMatrix).

#3 Updated by Tingjun Yang about 1 year ago

Mike just pointed out this is the refactored larsim code, which was not used by MicroBooNE MCC8 or ProtoDUNE MCC10.

Apologies for the false alarm.

We still need to fix this for ProtoDUNE MCC11.

#4 Updated by Gianluca Petrillo about 1 year ago

Something like:

EField = efield * (geo::Xaxis() + fEfieldOffsets).R();

should do, except that it assumes electric field along X axis.
To know whether that is correct, access to TPC geometry is required (geo::TPCGeo::DriftDir()), but the service provider interface is not optimised for that.

Also it is necessary to pay attention to the relative sign of the offset and of the drift direction, and the absolute sign of the correction as used upstream. Just saying.

#5 Updated by Tingjun Yang about 1 year ago

  • Assignee changed from Michael Mooney to Wesley Ketchum

#6 Updated by Michael Mooney about 1 year ago

Note that this does not impact MicroBooNE MCC8 or ProtoDUNE MCC10, as Tingjun mentioned, but also shouldn't impact ProtoDUNE MCC11, as it does not yet make use of the refactored larsim code.

The original implementation of this feature (authored by me) can be found here:

https://cdcvs.fnal.gov/redmine/projects/larsim/repository/revisions/develop/entry/larsim/LArG4/ISCalculation.cxx

The bug pointed out by Tingjun was introduced by Wes Ketchum, who has now been assigned the ticket. I think he has already created a feature branch with the fix.

#7 Updated by Lynn Garren about 1 year ago

Wes, can the change simply be merged with develop?

Lynn

#8 Updated by Lynn Garren about 1 year ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Since the feature branch was merged into larsoft v07_03_00 last week, I tested this change against the head of develop. The unit tests are fine except for ubana, which is broken for other reasons. This change is now in the head of develop and will be included in this weeks' larsoft release.



Also available in: Atom PDF