-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Synching ALERT fork with main branch #378
base: development
Are you sure you want to change the base?
Conversation
It should have been AHDC::adc (since corresponding HitReader also uses this bank), it was AHDC::tdc.
(and stop changing wire number internally to start from zero) (this requires removing a manual +1 offset for wire number in a perl script in GEMC)
Iss272 alert indexing
get latest changes
Fix of a merge error after commit f10c09e
PreCluster pairing routine in AHDC ClusterFinder
…r-layer list of hits in AHDC PreCluster Modified the hardcoded lookup superlayer indices to obtain the geometry parameters to calculate x, y.
Modifications for debugging/validation of index convention change.
…ector_simple" for Hit, and "H_simple", "h_simple" for KFitter to return the preexisting matrix emasurements; new matrix measurements have been implemented in the methods "getMeasurementNoise", "getVector", "H" and "h". Added new memebers "adc" and "phi" for the hit for measured ADC and wire phi position at z = 0 respectively.
Getting changes from Michael
common-tools/clas-detector/src/main/java/org/jlab/detector/base/DetectorDescriptor.java
Show resolved
Hide resolved
@@ -27,7 +27,9 @@ public Cluster(PreCluster precluster, PreCluster other_precluster) { | |||
_PreClusters_list.add(precluster); | |||
_PreClusters_list.add(other_precluster); | |||
this._Radius = (precluster.get_Radius() + other_precluster.get_Radius()) / 2; | |||
this._Z = ((other_precluster.get_Phi() - precluster.get_Phi()) / (Math.toRadians(20) * Math.pow(-1, precluster.get_Super_layer()) - Math.toRadians(20) * Math.pow(-1, other_precluster.get_Super_layer()))) * 300 - 150; | |||
|
|||
this._Z = ((other_precluster.get_Phi() - precluster.get_Phi()) / (Math.toRadians(20) * Math.pow(-1, precluster.get_Super_layer()-1) - Math.toRadians(20) * Math.pow(-1, other_precluster.get_Super_layer()-1))) * 300 - 150; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hardcoded numbers should be replaced with variables possibly defined from the database or in this case the geometry service
if (precluster.get_Super_layer() == super_layer && precluster.get_Layer() == layer && !precluster.is_Used()) { | ||
ArrayList<PreCluster> possible_precluster_list = new ArrayList<>(); | ||
|
||
double phi_mean = precluster.get_Phi() + 0.1 * Math.pow(-1, precluster.get_Super_layer()); | ||
double phi_mean = precluster.get_Phi() + 0.1 * Math.pow(-1, precluster.get_Super_layer()-1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment about hardcoded numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the "0.1" in question
switch (this.superLayerId) { | ||
case 0: | ||
case 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment about hardcoded numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments in the ATOF::add bank should be updated to be consistent with sector/layer starting from 1
@@ -23,21 +23,21 @@ private void fill_list(List<Hit> AHDC_hits, ArrayList<Hit> sxlx, int super_layer | |||
|
|||
public void findPreCluster(List<Hit> AHDC_hits) { | |||
ArrayList<Hit> s0l0 = new ArrayList<Hit>(); | |||
fill_list(AHDC_hits, s0l0, 0, 0); | |||
fill_list(AHDC_hits, s0l0, 1, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The List of Lists should be filled all at once to avoid looping 12 times through the hits
public int id; | ||
|
||
public float adcMax; | ||
public float timeRiseCFA; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this class is for general use, members' names should be more generic removing the CFA or CFD suffix.
Also the pulse "rise" and "fall" are usually referred to as leading and trailing edge that could be better names
public long timeStamp; | ||
public float fineTimeStampResolution; | ||
public static final short ADC_LIMIT = 4095; // 2^12-1 | ||
public float amplitudeFractionCFA; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the pulse specific variables should be local variables in the extract method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good catch, critical for thread safety
…tem, one to minimize track distance. Added functions to calculate phi for hit as a function of z and to calculate the hit "sign" i.e. the side of the wire the track passes.
…ers all hits to make a track candidate and uncommented the track finding code.
Kalman Filter fixes: constants and parameters
And the creation of BMT::wf (and conversion to BMT::adc) in the decoder was intended only as a working example, to be replaced with AHDC. |
Ok, after this is merged, then the Mode3 class in the decoder needs to be switched to the one AHDC wants to use. |
fix bad merge conflict
…r to not loop 12 more times than needed
Actually use the new waveform conversion approach for AHDC
Use DeepJavaLibrary to load a PyTorch model. Create a new bank AHDC_AI::Prediction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ATOF geometry should be untouched.
common-tools/clas-geometry/src/main/java/org/jlab/geom/detector/alert/ATOF/AlertTOFFactory.java
Outdated
Show resolved
Hide resolved
This reverts commit 3420bb9.
This is now required. |
Revert "leave internal indexing at zero, output at 1"
Add AI for ALERT trackfinding
Who can resolve the ATOF geometry conflict? |
No description provided.