Skip to content
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

All the Hit Reconstruction Optimizations Rory has worked on #1043

Merged
merged 20 commits into from
Aug 10, 2024
Merged

Conversation

rodwyer100
Copy link
Contributor

This is the first in likely a long set of messages required to get the code pretty enough to folks liking. I am submitting the reconstruction hps-java code w/ relevant testing and regular use steering files. The steering files to use will be the one with VSplit in the name as it has all the changes that have been validated to work.

@rodwyer100
Copy link
Contributor Author

I'll fix the conflicts myself in a bit. In the meanwhile it'd be helpful for you to make some of the comments you forsee me needing to implement. Also I intend on pushing the hpstr analysis scripts soon as well, but that can be later. I can put the scripts I used to run all the analysis made within the hps-mc framework, but that is in the future.

@cbravo135 cbravo135 changed the title All the Reconstruction Optimizations We Have in Mind. All the Hit Reconstruction Optimizations Rory has worked on Aug 5, 2024
Collection<ShapeFitParameters> doublePulse = twoPulseFitter.fitShape(rth, shape);
double doublePulseChiProb = doublePulse.iterator().next().getChiProb();
ShapeFitParameters Hello = doublePulse.iterator().next();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change this variable name to something descriptive and not Hello.

@cbravo135 cbravo135 requested a review from sarahgaiser August 9, 2024 22:00
Copy link
Collaborator

@sarahgaiser sarahgaiser left a comment

Choose a reason for hiding this comment

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

I trust Cam that he checked the functionality of this. It would be nice to have some updates in the formatting of the code to look prettier, but if you decide not to include those suggestions that is fine as well.

}

} // End of loop over seeds

// Finished finding clusters
return cluster_list;
}
//WILL RETURN THE LIST OF SPLIT CLUSTERS, THOUGH NOT CHECKED BY SIGNIFICANCE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this have to be all caps?

//WILL RETURN THE LIST OF SPLIT CLUSTERS, THOUGH NOT CHECKED BY SIGNIFICANCE
private ArrayList<List<LCRelation>> hasV(List<LCRelation> cluster){
ArrayList<Integer> vloc = new ArrayList<Integer>();
int minChan=1000000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you update the formatting in this part of the code so that we have whitespace around =, ||, &&, <, >, and other operators? This just makes it easier to read. Same for the loops and if statements:
for (int I=0; I < cluster.size(); I++) { ...
reads a bit better than for(int I=0;I<cluster.size();I++){

int minChan=1000000;
int maxChan=-1;
//CHANNELS AREN'T ORDERED PROPERLY, SO YOU HAVE TO ORDER THEM
for(int I=0;I<cluster.size();I++){
Copy link
Collaborator

Choose a reason for hiding this comment

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

usually, we use lower case indices i, j, k, ... in (nested) for loops.

@rodwyer100 rodwyer100 merged commit 8560e53 into master Aug 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants