Documentation / poetry improvements and fixes #773
Conversation
…mprove-optional-deps
|
ping @fschlueter - I hope you're happy with the changes. I think I have correctly described the way trigger times work as well in times.rst as well as the |
| Returns the absolute time of the trigger with respect to the beginning of the event, | ||
| i.e. the first time in the event where the trigger condition was fulfilled |
There was a problem hiding this comment.
The absolute time of the trigger with respect to the beginning of the event sounds like a contradiction by itself to me. If it relative to e.g., the station_time or event_time I would specify this.
|
A general comment/question. You are often always omitting the "." at the end of a sentence. I assume this is on purpose? Is there a PEP rule/suggestion on this? Similarly, you do not start sentences with a capitalised letter. Both of that I would do but I do not care much if we just try to be consistent with that in the future (or decide intentionally to be not consistent with that)? |
| which is used by both `electric field <NuRadioReco.framework.electric_field.ElectricField>` and `voltage trace <NuRadioReco.framework.channel.Channel>` classes. | ||
| The start of the trace is also accessible separately as the `trace_start_time <NuRadioReco.framework.base_trace.BaseTrace.get_trace_start_time>`. | ||
|
|
||
| Time delays are introduced by several hardware components. These time delays are often corrected for by folding/unfolding the complex transfer function (for an amp e.g. the measurement of the S12 parameter). |
There was a problem hiding this comment.
corrected for in this context sounds a bit confusing?
| To avoid this behavior we often use the following procedure, through the `NuRadioReco.modules.channelStopFilter` module: | ||
|
|
||
| We smoothly filter the first 5% and last 5% of the trace using a Tukey window function. This is a function that goes smoothly from 0 to 1. | ||
| To avoid rollover of the pulse, we add 128ns of zeros to the beginning and end of the trace. Steps 2) and 3) are performed by the channelStopFilter module | ||
| Both electric fields and channels have a trace_start_time variable. The get_times() function will automatically add the trace_start_time to the time array. | ||
| To avoid rollover of the pulse, we add 128ns of zeros to the beginning and end of the trace. |
There was a problem hiding this comment.
It is what the channelstopfilter does. Whether it is actually used in practice or not is up to the user, I think : )
There was a problem hiding this comment.
Yes, but how it is written within the docu suggests that it is the common/default behaviour. Maybe we can add this module to the list of modules which change the timing?
| The trace_start_times are all given relative to the station_time of the station the E-field or channel belongs to. The station_time is stored in an astopy.time.Time object for sub nanosecond precision on absolute times. | ||
| The trace_start_time itself is stored as a float. For simulations, the trace_start_time is relative to the vertex time, i.e., the time of the particle interaction. | ||
|
|
||
| The `trace_start_time <NuRadioReco.framework.base_trace.BaseTrace.get_trace_start_time>` is always relative to the station_time of the station the E-field or channel belongs to. The station_time is stored in an `astropy.time.Time` object for sub nanosecond precision on absolute times. |
| @@ -1,52 +1,66 @@ | |||
| overview of times | |||
| Overview of times | |||
There was a problem hiding this comment.
A section for the trigger time is missing.
I agree. Though I would like to point out that |
I was not referring to `times.rst' but the doc-strings in function definitions. |
|
|
||
| [tool.poetry.extras] | ||
| documentation = ["Sphinx", "sphinx-rtd-theme", "numpydoc"] | ||
| proposal = ["proposal"] | ||
| galacticnoise = ['pygdsm'] | ||
| ift_reco = ['nifty5', 'pypocketfft'] | ||
| muon_flux_calc = ['MCEq', 'crflux'] | ||
| RNO_G_DATA = ["mattak", "runtable", "pandas"] |
There was a problem hiding this comment.
Why removing runtable and pandas from the RNO_G_DATA dependencies?
There was a problem hiding this comment.
I mean, frankly, I don't remember. But runtable is not a public dependency so will fail (and the installation will error out) for some users (probably mainly RNO-G users that don't have their git profile set up via ssh), whereas reading RNO-G data is in theory possible for everyone with mattak only.
I suppose technically this could have been two PRs but no functionality should be affected.
pip install nuradiomc[documentation, proposal]to install the correct optional dependencies. Previously, optional dependencies were implemented as developer dependencies, and would never be installed duringpipinstallation.