Action #1120
Feature #1036: Implement energy resolution
Add GCTAEdispRMF class
| Status: | Closed | Start date: | 01/29/2014 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assigned To: |  Forest Florent | % Done: | 100% | |
| Category: | - | |||
| Target version: | Stage Florent | |||
| Duration: |
Description
At the moment energy resolution exporters from HESS use the RMF format.
Adding a GCTAEdispRMF wrapper class will allow using those in full likelihood fits (not XSPEC style).
Use GCTAAeffARF class as an example.
Test with first data challenge dummy RMF files for a few Crab runs.
Recurrence
No recurrence.
Related issues
History
#1
Updated by Deil Christoph almost 11 years ago
- Assigned To set to Owen Ellis
#2
Updated by Owen Ellis almost 11 years ago
- Status changed from New to In Progress
#3
Updated by Owen Ellis almost 11 years ago
- % Done changed from 0 to 30
In progress, current status can be found on the branch here: https://github.com/ellisowen/gammalib/tree/action_1120
I just have a couple of questions:
Would it be best here to copy the contents of the RMF or just store the RMF values?
How would it be best to implement the mc method here? Is some kind of random sampling from a step function already available somewhere in Gammalib which I could use?
#4
Updated by Owen Ellis almost 11 years ago
- Status changed from In Progress to Pull request
- % Done changed from 30 to 100
Hi Jürgen,
Please can you review this:
https://github.com/ellisowen/gammalib/tree/action_1120
I have implemented the unit tests for this, but haven’t yet done any high level testing of the new code.
#5
Updated by Deil Christoph almost 11 years ago
Did you commit the test/make_solidangle_image.py file by accident?
The easiest solution is to remove it in a new commit ... removing it by modifying the existing commits is possible, but not trivial.
#6
Updated by Owen Ellis almost 11 years ago
Yes, it seems I introduced that by accident in an earlier commit. Now removed in a new commit.
#7
Updated by Knödlseder Jürgen almost 11 years ago
I started to look into the code. There are obvious things that need to be corrected, I’ll go ahead and do this as much as possible. I keep you posted.
Generally, I would propose to move much of the energy boundary management things in the GRmf class instead of copying this into the GCTAEdispRmf class. GCTAEdispRmf should then just hold a GRmf member.
We obviously need also some interpolation methods, actually the mc method returns for example only the bin centres. What I typically do in this case is some randomization within the bin size. This is not perfect, but at least we get infinite precision values.
#8
Updated by Knödlseder Jürgen almost 11 years ago
By the way: I propose to rename to GCTAEdispRmf to be consistent with the naming conventions.
#9
Updated by Knödlseder Jürgen almost 11 years ago
- Status changed from Pull request to Feedback
I now merged all the stuff in and did some adjustments in GCTAEdispRmf. The energy boundary code in now at the GRmf level, and I added GRmf as a member to GCTAEdispRmf. This removes the need for carrying copies of the energy boundaries in the GCTAEdispRmf class.
I also linked GCTAEdispRmf to GCTAResponse so that the class is automatically instantiated if a RMF is available in the observation definition file. This works (is now a unit test for this in the CTA Python test).
I have not yet looked into the results of the mc method and the model access operator. But code compiles and all tests are fine, so we can close this issue for now.
#10
Updated by Knödlseder Jürgen over 10 years ago
- Target version deleted (
2nd coding sprint)
#11
Updated by Knödlseder Jürgen almost 10 years ago
- Target version set to Stage Florent
#12
Updated by Knödlseder Jürgen almost 10 years ago
- Assigned To changed from Owen Ellis to Forest Florent
Check where we stand and close the issue if the class is complete.
#13
Updated by Knödlseder Jürgen over 9 years ago
- Status changed from Feedback to Closed
- Remaining (hours) set to 0.0
Done