nu-radio / NuRadioReco

reconstruction framework for radio detectors of high-energy neutrinos
GNU General Public License v3.0
5 stars 3 forks source link

Vertex times #189

Closed daniel-zid closed 4 years ago

daniel-zid commented 4 years ago

Trigger times now include the vertex times

daniel-zid commented 4 years ago

Keep in mind that the trigger tests are going to fail because of the different times.

daniel-zid commented 4 years ago

we are already at a newer version. Can you update the changelog accordingly?

Why does the PA trigger sets the trigger time to the trace start time? Isn't there a specific location in the trace where the trigger is fulfilled?

No, there is not. There is a location where the phased trace fulfils the condition. When traces are phased, one must choose a reference position or a reference time, since the traces are moved around and the original timing is lost. In other words, the phased trace times do not correspond to any other channel times, in general. I suppose we could choose a convention, if you want. In any case, the delays that have triggered are already saved and they can be used to reconstruct the triggering phased trace.

If you have any preference let me know.

daniel-zid commented 4 years ago

we are already at a newer version. Can you update the changelog accordingly?

The master still says 1.0.1, right?

Why does the PA trigger sets the trigger time to the trace start time? Isn't there a specific location in the trace where the trigger is fulfilled?

cg-laser commented 4 years ago

no, it's at 1.1 (https://github.com/nu-radio/NuRadioReco/blob/master/NuRadioReco/__init__.py). Sry this is my fault, I forgot to properly update the changelox.txt when I did the release.

daniel-zid commented 4 years ago

The tests work perfectly on my machine but not on the build bot. I am lost. Could you take a look, @cg-laser?

anelles commented 4 years ago

Have you tried recompling the ray tracer on your machine? Execute setup.py again. Anna

On Mon, Feb 10, 2020, 14:37 daniel-zid notifications@github.com wrote:

The tests work perfectly on my machine but not on the build bot. I am lost. Could you take a look, @cg-laser https://github.com/cg-laser?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/nu-radio/NuRadioReco/pull/189?email_source=notifications&email_token=AC663BNGA5VQOTI4XJQHGADRCFKDDA5CNFSM4KQIY2W2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELIQ5OI#issuecomment-584126137, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC663BP6UJRNZLKBK3PZ74TRCFKDDANCNFSM4KQIY2WQ .

daniel-zid commented 4 years ago

Have you tried recompling the ray tracer on your machine? Execute setup.py again. Anna

Many many times

cg-laser commented 4 years ago

seems that you solved it? It was just the wrong NuRadioReco version? I'd suggest merging NuRadioReco first, then changing travis back to master, and then merging this branch.

cg-laser commented 4 years ago

ups I was confused. Hmm now we have this weird circular dependency between NuRadioMC and Reco...

cg-laser commented 4 years ago

please continue this PR at #194