Skip to content
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

Remove S2 tail cut from calibration modes #88

Merged
merged 3 commits into from
Dec 1, 2017
Merged

Conversation

coderdj
Copy link
Contributor

@coderdj coderdj commented Oct 26, 2017

We really need to do this

Copy link
Contributor

@pdeperio pdeperio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done in #86 by @sreichard, but I requested to remove it from there to have a separate PR like this.

@@ -52,6 +52,10 @@ class LowEnergyRn220(AllEnergy):

def __init__(self):
AllEnergy.__init__(self)

# S2Tails not used in calibration modes
self.lichen_list.pop(8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just delete the cut from the AllEnergy list above (like in #86)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. Technically it may also belong in "AllEnergy" since it should work the same at all energy scales.

Copy link
Contributor

@pdeperio pdeperio Oct 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see sorry, missed that this was in LowEnergyRn220.

Maybe should move below:

self.lichen_list[4] = S2SingleScatterSimple()

in case future people pop more and affect the indexing of the cuts below.

@feigaodm
Copy link
Member

Why do we need to remove this cut from the list? Can you remind me by adding a note. Same holds for pre-S2 junk cut.

@pdeperio
Copy link
Contributor

@skazama's note reported a loss of acceptance in calibration data, I guess because the cut was ultimately tuned for background data.

But if I remember correctly, @coderdj @ramirezdiego, in e.g. SR0, you evaluated this for AmBe, so couldn't you also provide a different tuning for each source?

@pdeperio
Copy link
Contributor

PreS2Junk cut is not changing as far as I know. Or did you mean to add notes for it too?

@pdeperio pdeperio added this to the v1.1.0 milestone Nov 2, 2017
@coderdj
Copy link
Contributor Author

coderdj commented Nov 3, 2017

@pdeperio the reason this was tuned first on AmBe in SR0 was because I hadn't realized yet we could use blinded background for it as long as we didn't include cs2 or make any controversial spectra. Once that method got established there was no reason to use AmBe anymore and it was dropped.

I think it's not worth the time to tune this for each source. You can apply it as-is if you only want very clean events, but this should be done knowingly.

@feigaodm
Copy link
Member

feigaodm commented Nov 3, 2017

Sorry, forgot to reply. Both cuts are very useful to validate the AC analysis, especially pre-S2 junk. I will update the test to see if we can have a reliable prediction without them.

@coderdj
Copy link
Contributor Author

coderdj commented Nov 3, 2017

@feigaodm I don't understand. Why do you need to validate AC in calibration modes? This will get super tricky because of the correlation effect with tails of big S2s. With the tail cut in background mode lone S1/S2 should not come from an S2 tail but from something else.

@feigaodm
Copy link
Member

@coderdj We need AC model in Rn220 calibration data when Qing et.al performs the band fitting, hence AC model must be validated on calibration data as well. To do this, we need pre-S2 junk cut, otherwise there will be a lot of junk events which can not be modeled by AC analysis. This is especially true for events with s2 below the threshold of 200PE, junks from previouse event can bias the analysis a lot.

@pdeperio pdeperio modified the milestones: v1.1.0, v1.3.0 Nov 21, 2017
Copy link
Contributor

@pdeperio pdeperio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if AC model has this cut applied, wouldn't it be incomparable to calibration data that does not have it applied? Or are @skazama's figures here and here enough to show that this is not the case?

Also, if we decide to remove, @coderdj did you want to remove from SR0 too?

@feigaodm
Copy link
Member

I guess it's better to modify the cuts instead of removing them.

@pdeperio The cuts has been considered in both prediction and validation stage so there is no mis-match due to those cuts.

@coderdj
Copy link
Contributor Author

coderdj commented Nov 30, 2017

@pdeperio I removed from SR0 also. @feigaodm I think better to not include and people can specifically add cut as needed. We haven't been using this cut for selecting the datasets used for band fitting but it might be appropriate in other cases.

@feigaodm
Copy link
Member

Yeah, I agree. I talked to @skazama and confirm that the acceptance loss is very bad for NG data only. For Rn220 data, I don't think the effect is very big, maybe 10-20% level.

@skazama
Copy link
Contributor

skazama commented Nov 30, 2017

From these plots, AmBe and NG and Rn220 and BG, the acceptance loss for Rn220 is just 12% for this cut. For AmBe, it is 13% loss.

@sreichard sreichard merged commit 7e62e20 into master Dec 1, 2017
@sreichard sreichard deleted the coderdj-patch-1 branch December 1, 2017 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants