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

Sideband unblinding for AC+Wall model #199

Merged
merged 12 commits into from
Feb 9, 2018
Merged

Conversation

pdeperio
Copy link
Contributor

@pdeperio pdeperio commented Jan 12, 2018

This PR unblinds the sideband below the red line and cs1 = 200 pe in the following image, first defined in #168:

@feigaodm feigaodm self-requested a review January 12, 2018 17:44
@feigaodm
Copy link
Member

It's too difficult to review it:) Can you try to modify it this way:
lowe_er = "sth"
multiple_scatter = 'largest_other_s2 > 200'
double_e_capture
...
blinding_cut = lowe_er | multiple_scatter | double_e_capture

Copy link
Member

@feigaodm feigaodm left a comment

Choose a reason for hiding this comment

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

besides that, we shall unblind signals with S2 between 150PE and 200PE

@pdeperio
Copy link
Contributor Author

@feigaodm I've implemented your requests.

Also, just additionally unblinded s2<200 for cs1<20 (s2<150 is already unblinded), which I think is what you want as shown in the following figure, right?

blinding_highe

@coderdj
Copy link
Contributor

coderdj commented Jan 25, 2018

I like the reorganization but considering the confusion we had recently do we want to consider making it more clear that these options shouldn't be touched?

Like, we had more than one analyst (non-maliciously) modifying something called 'blinding_cut'. So that strategy was already a cause for confusion apparently. Now we have a bunch of "unblind_*" options... which at first glance are far more confusing.

Do you think it's possible to put all the blinding options in a separate section or file clearly delineated as 'do not touch' and not modifiable in override via hax.init?

@pdeperio pdeperio mentioned this pull request Jan 30, 2018
@pdeperio pdeperio requested a review from coderdj February 8, 2018 20:24
@feigaodm feigaodm merged commit 7ce30ca into master Feb 9, 2018
@feigaodm feigaodm deleted the sr1_sideband_unblinding branch February 9, 2018 03:42
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.

None yet

3 participants