Enable modifications of the RNO-G detector description

[fschlueter]

I all,

with this PR I want to start a discussion on how we will support the feature to modify the RNO-G detector description "on-the-fly". The feature is useful for systematic studies and in our efforts to improve the detector description. On example: We want to rotate the antenna orientations to see the effect of this rotation to the simulation of the galactic emission. In the future it will come in handy to allow to replace the response function of a channel with something different.

This PR adds a very simple function to the detector class which should (untested!) allow you to replace a certain value in the detector description. It is not very user friendly in the sense that you really need to know what you have to do. We could think of more specific user friendly interfaces. It could be a good possibility to create an inherent class which hosts all these modification interfaces we envision.

Any suggestions / comments / opinions?

[philippwindischhofer]

Hi @fschlueter thanks for starting the discussion on this topic! Below are a few quick comments and questions from my side. If I can think of anything else, I will post another comment.

Station geometry storage and bookkeeping

1. Is the long-term plan to store the station / channel geometry information also in the hardware DB? (I can already see some geometry-related collections filled, so I guess the answer is "yes"?) If so, I would advocate for removing the current json files as soon as the DB-based solution is fully available.
2. The DB has the nice "interval of validity" (IOV) mechanism to allow multiple geometries to be available for non-overlapping times. When we iterate on the geometry calibration later this year (and in the longer-term future), we might have to update the geometry as new calibration data becomes available. If our best estimate for the station geometry changes over time, I wonder if the IOV is the best mechanism to handle that? E.g. we might want to keep multiple past calibration results around to easily check how reconstruction / simulation results change. One way to implement this would be to build a poor-man's commit-tree by storing for each station geometry also a "parent" that points to the geometry that it corrects / updates. (I don't know if having multiple overlapping IOVs would break things elsewhere.)

On-the-fly updates of station geometry

3. I guess we might want to support two separate interfaces: an "expert" one that allows the user to modify all degrees of freedom in an unconstrained manner as they please; and a "user" one that provides a more high-level interface. In the latter case, only a few "knobs" would be exposed that internally map onto antenna positions. For example, if we decide to go for the eigenvector parametrization of geometry calibration results that I presented at the virtual collaboration meeting, the exposed DOFs would be the nuisance parameters corresponding to the eigenmodes.

Visualization

4. Do we already have some centralized code to visualize a given station geometry? I believe this could be worthwhile to build intuition for how a particular update to the geometry actually changes the situation in the ice. Right now, I think people write their own scripts (I have one, the calibration code has one, and I think @MatthewMarsee also has one), and it might be worth harmonizing these.

[fschlueter]

Hey @philippwindischhofer some quick answers:
To 1): Yes and Yes :)
To 2): We have anticipated for what you describe and the DB / NuRadio class already accommodates for this. A longer explanation is here but the short version is: With the database_time you can select a measurement of e.g. the channel position which was primary at that specific time while in the mean time with have specified a new, supposedly better measurement, as the primary one.
To 3): Sounds reasonable and I guess we can add the user-friendly interface along the road as we need it. Should be but that in a dedicated class though?
To 4): I am not terrible aware of all the different visualisation codes neither (maybe @sjoerd-bouma can help us out here). But I remember that we have a detector browser viewer?

[philippwindischhofer]

Hi @fschlueter, thanks for your rapid answer!

2. Brilliant!! I wasn't aware of this.
3. Yes, I would put all that into dedicated classes to keep it nice and separate. A future GeometryCalibration class would then take as inputs the database_time and values for the nuisance parameters, and return the properly calibrated detector geometry.

[fschlueter]

Yes, I would put all that into dedicated classes to keep it nice and separate. A future GeometryCalibration class would then take as inputs the database_time and values for the nuisance parameters, and return the properly calibrated detector geometry.

Do you plan to perform calibrations "on-the-fly". I thought the idea is to run a calibration analysis and put the results of this in the DB

[philippwindischhofer]

Yes, the calibration would be run once, recorded in the DB, and a particular calibration retrieved via the database_time mechanism.

As part of the calibration we will get uncertainties on the station geometry. If we want to propagate the geometry uncertainties into a simulation / reconstruction (especially at the beginning, where the uncertainties will be large), we need a mechanism to sample the space of allowed station geometries. That part, I think, would profit from being done "on the fly" (since it is fast), instead of storing many explicit variations of the geometry in the DB.

[sjoerd-bouma]

@philippwindischhofer Re 4) - there's a nice interactive 3D visualization of NuRadio detectors (see the documentation for some more information on how to use it). It probably needs an update in order to also support the RNO-G detector, though; right now it only works with jsons, and the interface is slightly outdated.

[fschlueter]

Yes, the calibration would be run once, recorded in the DB, and a particular calibration retrieved via the database_time mechanism.

As part of the calibration we will get uncertainties on the station geometry. If we want to propagate the geometry uncertainties into a simulation / reconstruction (especially at the beginning, where the uncertainties will be large), we need a mechanism to sample the space of allowed station geometries. That part, I think, would profit from being done "on the fly" (since it is fast), instead of storing many explicit variations of the geometry in the DB.

I see. Probably we should think of a smart way to allow shifting of positions and orientations.

[shallmann]

Thanks for starting this, Felix!

• The detector validity is not affected by this, i.e. running a detector.update() would not re-set the values to the original. Maybe we need to keep this in mind (and might want to introduce a force_update or a is_modified somewhere to check if the detector is modified or not and be able to get back to the original if need be.

• Having the possibility to modify detector descriptions will be very useful for sure. In addition to the general modify_channel_description function we can add more specialised functions to shift/rotate individual channels as we need them.

• I like the suggestion @philippwindischhofer to be able to automatically randomly shift detector components by their allowed uncertainties. I guess not but: Are there already fields for uncertainty in the database?

• If we have no other volunteers, I can look into updating the detector visualisation next week.

[sjoerd-bouma]

Tangentially to Steffen's comment and as was suggested by Christian in #616, I think it's important to be able to distinguish a modified detector from an unmodified detector. If we don't introduce a separate 'ModifiedDetector' class I think it would be good to at least have a (one-time) warning issued when a user first modifies the detector, and maybe an is_modified property as Steffen suggests to keep track of this. Then in terms of being able to store/keep track of modified values I guess exporting to json still works?

[philippwindischhofer]

Hey @fschlueter and @shallmann,

I see. Probably we should think of a smart way to allow shifting of positions and orientations.

I like the suggestion @philippwindischhofer to be able to automatically randomly shift detector components by their allowed uncertainties.

For at least some cases (calibration being one of them) a proper treatment of the uncertainties will be a bit more complicated than simply "randomly wiggle antennas around independently". Rather, each uncertainty component will move around a set of antennas in a particular way. This is what I called "eigenmodes" earlier.

I guess not but: Are there already fields for uncertainty in the database?

I don't think there currently are---but @fschlueter or @jakobhenrichs would know for sure. Given the above, I think an important aspect will be to decide on the format we choose to store the "modifications" in. If we want this to go into the DB, it should be sufficiently general to cover all our future use cases.

Hey @sjoerd-bouma,

Tangentially to Steffen's comment and as was suggested by Christian in #616, I think it's important to be able to distinguish a modified detector from an unmodified detector.

I would even go one step further and, instead of just tagging a detector geometry as "unmodified" and "modified", give them more informative "tags". There would be a single "nominal" detector geometry that represents the best-estimate of the actual as-deployed instrument, and then there would be several named variations around that nominal case.

If we don't introduce a separate 'ModifiedDetector' class I think it would be good to at least have a (one-time) warning issued when a user first modifies the detector, and maybe an is_modified property as Steffen suggests to keep track of this. Then in terms of being able to store/keep track of modified values I guess exporting to json still works?

I don't have any opinion on whether a ModifiedDetector class is the way to go, or whether the current Detector class should hold this functionality. I would just advocate to make sure that the interface is the same for both, e.g. querying channel positions etc. should work in exactly the same way for both, so that we can use a modified detector in all locations where we can use an unmodified one. That's the essential feature that will make rigorous propagation of geometry uncertainties possible.

[shallmann]

For at least some cases (calibration being one of them) a proper treatment of the uncertainties will be a bit more complicated than simply "randomly wiggle antennas around independently". Rather, each uncertainty component will move around a set of antennas in a particular way. This is what I called "eigenmodes" earlier.

I didn't fully appreciate the meaning of 'eigenmodes' earlier, yes you are right. We need to agree on which uncertainties and correlations / eigenmodes we need in the database. Independent per-channel uncertainties would have been a no-brainer to add to the database, eigenmodes may need a bit more thought.

Given the above, I think an important aspect will be to decide on the format we choose to store the "modifications" in. If we want this to go into the DB, it should be sufficiently general to cover all our future use cases.

I would even go one step further and, instead of just tagging a detector geometry as "unmodified" and "modified", give them more informative "tags". There would be a single "nominal" detector geometry that represents the best-estimate of the actual as-deployed instrument, and then there would be several named variations around that nominal case.

1. We intended the database to hold new "best fit" positions, and (eventually) uncertainties/eigenmodes after each iteration of calibrating the detector (each iteration retrievable by specfying a database_time when initialising the detector object). I think this is what we definitely want in the official database.
2. Another use case is to deliberately shift/delay/rotate a set of channels for individual studies. We should have a functionality to allow for that by passing a modification dict or applying some modification functions. I think this is what Felix already had in mind when starting this thread.
3. We can discuss on wether (and how) we need official fixed named modification sets stored centrally (which cannot be regenerated by a function using the official best fit database values and uncertainties).
4. A simple variable to tell if a detector was manipulated will be useful irrespective of that.

I don't have any opinion on whether a ModifiedDetector class is the way to go, or whether the current Detector class should hold this functionality. I would just advocate to make sure that the interface is the same for both, e.g. querying channel positions etc. should work in exactly the same way for both, so that we can use a modified detector in all locations where we can use an unmodified one. That's the essential feature that will make rigorous propagation of geometry uncertainties possible.

No strong opinion either, but if we opt for a separate ModifiedDetector class this should derive from the unmodified detector of course to ensure all queries still work the same. Keeping both a modified and an unmodified detector alive (which afaik may be not possible with two normal detectors since they are a singleton?!?) would allow for direct comparisons between the two, if there is a use case

[philippwindischhofer]

Independent per-channel uncertainties would have been a no-brainer to add to the database, eigenmodes may need a bit more thought.

Yep! Although, if we find a good way of keeping the eigenmodes in the DB, then independent per-antenna uncertainties are just a special case :-)

I fully agree with your points 1-4!

Keeping both a modified and an unmodified detector alive (which afaik may be not possible with two normal detectors since they are a singleton?!?) would allow for direct comparisons between the two, if there is a use case

Indeed! I think this is a good argument in favor of a ModifiedDetector. And if we generate the ModifiedDetector through a call to the original Detector like Detector.modify(mod), then we can also easily loop over all variations (including the nominal case).

[shallmann]

• If we have no other volunteers, I can look into updating the detector visualisation next week.

Added as much as possible given the current state of the RNO-G detector in this PR: #690

[philippwindischhofer]

Hi again,

I wanted to come back to this and open up another topic for discussion. If we are going to have the detector class support alternative geometries, it might be useful to be able to read those in from a file, similar to how detector descriptions may currently be specified in a json file.

I had a go at a possible schema here. I can provide an example calibration output if interested. As you can see, this follows the current NuRadioMC schema quite closely (is this formally specified somewhere?).

Sticking with json should also allow us to keep this inside the DB if we want.

What do you think?

[fschlueter]

Hi @philippwindischhofer,
in the future (and actually the future is now :) ) we will use a detector description queried from the hardware database with the new rnog_detector class. The position of each channel is stored in the DB and the idea was to store results of a calibration there too. So far there are no fields for uncertainties but this could be added easily.

[fschlueter]

Hi all, I will need to go through that entire thread at some point (sorry for not having done so already and coming back to this). However, I wanted to let you know that I started an implementation of a ModDetector class in the rnog_trigger branch.

[shallmann]

I started taking the ModDetector class Felix started with from the rnog_trigger branch in order to develop that further.

[shallmann]

To me the way the response_chain and total_response are stored do not look ideal, it seems that things are stored twice and it is obscure which functions internally use which, and not straightforward to keep them in sync.

In an ideal world, a function like the add_response above should be enough, but det.get_time_delay() does not look at the total response, so the modifications seem necessary at two places.

Maybe I am missing something obvious how this was supposed to work?

[shallmann]

This is a demo of what the (still work in progress) added functions do...