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.

Sign up for GitHub

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

Jump to bottom

Refactor cities flow #842

Merged
merged 80 commits into from
May 14, 2024
Merged

Refactor cities flow #842

merged 80 commits into from
May 14, 2024

Conversation

gonzaponte
Copy link
Collaborator

This PR introduces a major change in the organization of the IC cities structure. Details here and here.

Since this is a major change in the repository, we will be moving to version 2.0.0 when this PR is merged.

martinperezmaneiro
martinperezmaneiro reviewed Dec 11, 2023
View reviewed changes
Copy link
Contributor

@martinperezmaneiro martinperezmaneiro left a comment

Choose a reason for hiding this comment

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

In general, I just have some minor comments or questions. The big picture of this PR and the new structure of the flow is clear reading the code

@gonzaponte gonzaponte force-pushed the refactor-flow branch 2 times, most recently from 7e0ed15 to 1b5941d Compare April 30, 2024 09:31
martinperezmaneiro
martinperezmaneiro approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@martinperezmaneiro martinperezmaneiro left a comment

Choose a reason for hiding this comment

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

Latest commits gave the final shape to the refactor of the cities, the code is clear and understandable. Great work, approved!

gonzaponte added 23 commits May 13, 2024 18:07
@gonzaponte
Cosmetics
8a50cb3
@gonzaponte
Refactor compute_and_write_tracks
d535189
@gonzaponte
Add hits_merger and hits_thresholder to components
1c6b2ed
@gonzaponte
Add sipms_as_hits
d49fec1
@gonzaponte
Add sophronia
c872c74
@gonzaponte
Handle corrected energy in threshold_hits
dac0865
@gonzaponte
Handle corrected energy in merge_nn_hits
c18c6d5
@gonzaponte
Replace cluster.empty with cluster.zero. TODO: user cluster.empty
9f16826
@gonzaponte
Redesign esmeralda
8a0ac3f
@gonzaponte
Update esmeralda tests
9c683ec
@gonzaponte
Update esmeralda config
1f167cd
@gonzaponte
Generalize hits_and_kdst_from_files for different groups and nodes
2f31159
@gonzaponte
Redesign beersheba
aee6d82
@gonzaponte
Remove summary-related functionality from beersheba
1462804
@gonzaponte
Transform hitc into df
7906fd0
@gonzaponte
Remove cdst_from_files
4e9bb19
@gonzaponte
Remove unused imports
7d214d0
@gonzaponte
Update Beersheba config
d4af1bf
@gonzaponte
Update beersheba fixture
a9fcc6f
@gonzaponte
Remove low_th checks in esmeralda tests
c843232
@gonzaponte
Rename tests
dce68d3
@gonzaponte
Parametrize some tests
77494dc
@gonzaponte
Check also corrected energy in hits_functions_test
b800807
gonzaponte and others added 27 commits May 13, 2024 18:07
@gonzaponte
Update test files
7fb1438
@gonzaponte
Update isaura test
9f33dca
@gonzaponte
Fix beersheba test
1b99de2
@gonzaponte
Remove duplicate make_hits in Sophronia
09a873e
@gonzaponte
Make sure esmeralda doesn't drop events
ab460c8
@gonzaponte
Adapt esmeralda test
8313ca7
@gonzaponte
Make compute_and_write_tracks_info a sink
89b0f46
@gonzaponte
Replace penthesilea with sophronia in all_cities test
5c6a759
@gonzaponte
Rename some_cities -> online_cities
9d97fa3
@gonzaponte
Skip test_esmeralda_exact_result temporarily
5869c08
@jwaiton @gonzaponte
Remove detector_db being defaulted to 'new'
019f020
@jwaiton @gonzaponte
Add detector_db's argument explicitly in test
a35806d
@jwaiton @gonzaponte
Add test to ensure that detector_db is required
06b6cb0
@jwaiton @gonzaponte
Add detector_db explicitly to config files
b5b8efb
@gonzaponte
Make hit correction optional in Sophronia
18fa019
@gonzaponte
Add hit corrections to Beersheba
d0d77c2
@gonzaponte
Make hit corrections in esmeralda non-optional
bccea43
@gonzaponte
Update test files
3b50997
@gonzaponte
Sort hits before writing them in Esmeralda
e6eb007 
The networkx library does not determine the order of the output. This
translates into a random order of the hits table, which is irrelevant
for data analysis, but messes up the comparison between tables.
@gonzaponte
Recover test_esmeralda_exact_result
b92f8e9
@gonzaponte
Implement test_sophronia_filters_events_with_only_nn_hits
f855f01
@gonzaponte
Update test file for Isaura
8658e13
@gonzaponte
Improve Sophronia's docstring
5d6e1cf
@gonzaponte
Improve Esmeralda's docstring
146e4aa
@gonzaponte
Clean up unused imports
ed7445a
@gonzaponte
Improve Isaura's docstring
feb938d
@gonzaponte
Improve Beersheba's docstring
7e32798
@carhc carhc merged commit 706d338 into next-exp:master May 14, 2024
1 check passed
@gonzaponte gonzaponte deleted the refactor-flow branch May 14, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinperezmaneiro martinperezmaneiro martinperezmaneiro approved these changes

@jahernando jahernando Awaiting requested review from jahernando

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gonzaponte @martinperezmaneiro @carhc @jwaiton