-
Notifications
You must be signed in to change notification settings - Fork 35
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
Including a basic numerical radiopropa tracer #271
Conversation
ac0f6ba
to
1d25ed2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @boboeyen I went through all requested code changes. Looks pretty good. I added a few comments in-line.
radiopropa ice model, so it can be used in NuRadioMC. For example | ||
|
||
def get_ice_model_radiopropa(self): | ||
scalar field = radiopropa.New_IceModel(*args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this example args
is not defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this because depending on which icemodel you use or module you add the arguments are different and I didn't wanted it to be to specific but I don't know how to make this make clear that these things are placeholders for the real arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Bob, a few more comments inline.
@@ -1629,6 +1630,31 @@ def set_start_and_end_point(self, x1, x2): | |||
self.__x2 = np.array([X2r[0], X2r[2]]) | |||
self.__logger.debug("2D points {} {}".format(self.__x1, self.__x2)) | |||
|
|||
def use_optional_function(self, function_name, *args, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice implementation! But I'm wondering, can't we put this function into the base_class and then make all other ray tracer inherit from this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes indeed, Christoph also mentioned it and I had the same idea. I suggested to make a issue for this to keep thing a bit seperate. I'll work on it after this is merged :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, why not do it directly here? It it works it should be just a few more lines of code change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if we want to inherit from the base_class I would like to check it and make sure that everything is done right, also for set_start_end_end_point and other function inheritance could be an option I think. So I think working with a clean slate would be more clear and unclutter things. But we can discuss it tomorrow.
Looks like all my complaints were addressed :-) |
Makes channelSignalReconstructor store nosie RMS in channel parameters
A new numerical ray tracer is implemented in NuRadioMC to enable the investigation of non-exponential ice models. This basic tracer relies on radiopropa which will soon have a new release adapted to work with this tracer. The ray tracer works in a iterative way for now, a new minimiser will come later to speed up the process for certain ice models
To facilitate radiopropa, the medium implementation has also changed at the back and, but can be used in the front end as before. Implementing new ice models will have to be done in a new way and preferably in both NuRadioMC and radiopropa if you want to use this ray tracer.