change internal looping in NuRadioMC #222
Conversation
|
Looks good so far. Can you add a has_shower method to the event class to check if there is already a shower with a given ID? |
…elay and to convert individual efields
…which is because of a bugfix in the channelSignalReconstructor
…to event generator
allow for a different bandpass filter per channel
NuRadioReco/framework/sim_channel.py
Outdated
| logger = logging.getLogger('channel') | ||
|
|
||
|
|
||
| class SimChannel(NuRadioReco.framework.base_trace.BaseTrace): |
There was a problem hiding this comment.
I think it would be better to inherit from Channel
There was a problem hiding this comment.
yes that would be smarter. I'll change it
| @@ -31,10 +35,34 @@ def get_simulation_weight(self): | |||
| def set_simulation_weight(self, simulation_weight): | |||
| self.__simulation_weight = simulation_weight | |||
|
|
|||
| def serialize(self, mode): | |||
| base_station_pkl = NuRadioReco.framework.base_station.BaseStation.serialize(self, mode) | |||
| def iter_channels(self): | |||
There was a problem hiding this comment.
These channel are the same as in the Station class, so I would put them into the BaseStation to avoid duplication
There was a problem hiding this comment.
I don't understand this comment.
There was a problem hiding this comment.
Both the Station and the SimStation class now have a iter_channels method that does the same. So it would be better to define this once in the BaseStation class
There was a problem hiding this comment.
they are not exactly the same. The identifiers for the channels are different. We could move the data structure into the basestation and also the iter_channels function. Also the object itself is different. One is a SimChannel and the other one is a Channel.
So I fear it might get more confusing than helping to move part of the code to the base station.
There was a problem hiding this comment.
Hmm, I see. Hard to say which solution is better. So I'm fine with either. Have you tested if Channel and SimChannel are similar enough that you can use modules like the filter and resampler on both (by just passing the SimStation instead of the Station)? I think that would be very useful
There was a problem hiding this comment.
SimChannel inherits from Channel. The only difference is that it has two more identifiers, the ray tracing and shower id. So all modules that run on the channel should also run on the simchannel. I'm actually using it in NuRadioMC .
There was a problem hiding this comment.
Great, then I don't have anything to complain about any more ;-)
|
@cg-laser per code cleaning up, we should try to merge this. @christophwelling you would need to accepted the changes in order to make is mergable. |
|
Please put the iter_channels method from the Station and SimStation class into the BaseStation class, then I think this can be merged. |
|
The SouthPole test will fail because of the random seeds changes. So I need to update this. But before that, I need to merge the corresponding NuRadioMC changes into the internal-looping branch. For this I'm waiting on @anelles feedback/approval on the NuRadioMC pull request (the noise temperature one). |
implement that trigger modules accept a different threshold per channel
corresponding PR to nu-radio/NuRadioMC#208