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

Rollback for GRID support #52

Merged

Conversation

HealthyPear
Copy link
Member

@HealthyPear HealthyPear commented Jun 4, 2020

When I added protopipe.pipeline.utils.prod3b_array I made some changes that influenced the grid interface code.
Since at the time the GRID support was put on hold, I didn't even think that it could be a problem.
Indeed since the 2 repos are de facto separated, there was no automatic test to catch this...

The following changes have been rolled back now:

  • removed the camera ID list argument in the general analysis configuration file because that function guesses the properties of the Prod3b array used automatically.
    I added back that argument the must be filled by the user.
    One can put also all the cameras, since protopipe.pipeline.utils.prod3b_array will take care of it anyway (making that argument a dummy one).
  • same thing for the command line argument in write_dl1.py,
  • energy estimation command line argument was set to boolean, now it's again a string (this I don't know why was changed actually)

Other changes:
I also added a comment about using absolute paths when defining the simtel files lists to be used.
This is useful since I plan to enable the use of a container and the paths could be different from my personal setup.

Note: we should check if something like protopipe.pipeline.utils.prod3b_array already exists in ctapipe for Prod3b, otherwise it would be wise to add it, at least for Prod5 (issue to be opened soon to be sure).

@HealthyPear HealthyPear added fix A PR that fixes a bug or a wrong behaviour. grid labels Jun 4, 2020
@HealthyPear HealthyPear added this to the Release 0.3 milestone Jun 4, 2020
@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #52 into master will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #52      +/-   ##
=========================================
- Coverage    0.40%   0.39%   -0.01%     
=========================================
  Files          20      20              
  Lines        2232    2268      +36     
=========================================
  Hits            9       9              
- Misses       2223    2259      +36     
Impacted Files Coverage Δ
protopipe/pipeline/utils.py 0.00% <0.00%> (ø)
protopipe/scripts/write_dl1.py 0.00% <ø> (ø)
protopipe/pipeline/event_preparer.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c46571c...ac7bd3c. Read the comment docs.

Pipeline features and enhancements automation moved this from In progress to Reviewer approved Jun 9, 2020
@HealthyPear HealthyPear merged commit 8b35ee8 into cta-observatory:master Jun 9, 2020
Pipeline features and enhancements automation moved this from Reviewer approved to Done Jun 9, 2020
@HealthyPear HealthyPear deleted the rollback-grid_support branch June 9, 2020 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A PR that fixes a bug or a wrong behaviour. grid
Development

Successfully merging this pull request may close these issues.

None yet

2 participants