{"action":"created","issue":{"url":"https://api.github.com/repos/alisw/AliPhysics/issues/4010","repository_url":"https://api.github.com/repos/alisw/AliPhysics","labels_url":"https://api.github.com/repos/alisw/AliPhysics/issues/4010/labels{/name}","comments_url":"https://api.github.com/repos/alisw/AliPhysics/issues/4010/comments","events_url":"https://api.github.com/repos/alisw/AliPhysics/issues/4010/events","html_url":"https://github.com/alisw/AliPhysics/pull/4010","id":292655743,"number":4010,"title":"[WIP] EMCal Embed: Cent selection with embedded event","user":{"login":"raymondEhlers","id":1571927,"avatar_url":"https://avatars0.githubusercontent.com/u/1571927?v=4","gravatar_id":"","url":"https://api.github.com/users/raymondEhlers","html_url":"https://github.com/raymondEhlers","followers_url":"https://api.github.com/users/raymondEhlers/followers","following_url":"https://api.github.com/users/raymondEhlers/following{/other_user}","gists_url":"https://api.github.com/users/raymondEhlers/gists{/gist_id}","starred_url":"https://api.github.com/users/raymondEhlers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/raymondEhlers/subscriptions","organizations_url":"https://api.github.com/users/raymondEhlers/orgs","repos_url":"https://api.github.com/users/raymondEhlers/repos","events_url":"https://api.github.com/users/raymondEhlers/events{/privacy}","received_events_url":"https://api.github.com/users/raymondEhlers/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2018-01-30T05:15:45Z","updated_at":"2018-02-07T20:37:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/alisw/AliPhysics/pulls/4010","html_url":"https://github.com/alisw/AliPhysics/pull/4010","diff_url":"https://github.com/alisw/AliPhysics/pull/4010.diff","patch_url":"https://github.com/alisw/AliPhysics/pull/4010.patch"},"body":"__Work in Progress! Please do not commit!__\r\n\r\nPreviously, good embedded events were lost when the internal event\r\nwas outside of, for example, the desired centrality range. Now,\r\nthe user should check for\r\n`AliAnalysisTaskEmcalEmbeddingHelper::GetIstance()->IsGoodEvent()`\r\nto ensure that the internal event passes event selection and that\r\nthe embedded event was embedded successfully. If the internal event\r\ndid not pass event selection, the embedded event is saved for the\r\nnext internal event.\r\n\r\nThe code should be ready for code review, but it is still undergoing some\r\ntesting. @aiola - Would you mind reviewing this code? I think this simple solution is\r\nfine, but I would appreciate another set of eyes which is familiar with the embedding\r\nframework!"},"comment":{"url":"https://api.github.com/repos/alisw/AliPhysics/issues/comments/363902402","html_url":"https://github.com/alisw/AliPhysics/pull/4010#issuecomment-363902402","issue_url":"https://api.github.com/repos/alisw/AliPhysics/issues/4010","id":363902402,"user":{"login":"raymondEhlers","id":1571927,"avatar_url":"https://avatars0.githubusercontent.com/u/1571927?v=4","gravatar_id":"","url":"https://api.github.com/users/raymondEhlers","html_url":"https://github.com/raymondEhlers","followers_url":"https://api.github.com/users/raymondEhlers/followers","following_url":"https://api.github.com/users/raymondEhlers/following{/other_user}","gists_url":"https://api.github.com/users/raymondEhlers/gists{/gist_id}","starred_url":"https://api.github.com/users/raymondEhlers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/raymondEhlers/subscriptions","organizations_url":"https://api.github.com/users/raymondEhlers/orgs","repos_url":"https://api.github.com/users/raymondEhlers/repos","events_url":"https://api.github.com/users/raymondEhlers/events{/privacy}","received_events_url":"https://api.github.com/users/raymondEhlers/received_events","type":"User","site_admin":false},"created_at":"2018-02-07T20:37:21Z","updated_at":"2018-02-07T20:37:21Z","author_association":"CONTRIBUTOR","body":"Hi @aiola,\r\n\r\n@jdmulligan and I discussed this for quite a while today, but I think we've found something that we're starting to be happy with. Here is what we propose:\r\n\r\n- Circle back around to similar to my original implementation and apply event selection in the embedding helper via AliEventCuts (disabled by default)\r\n    - Since we apply event cuts here, no need to play games with increment and decrementing the current event. If the internal event doesn't pass the selection, then it we just don't embed anything, and we set a property that the embedded event isn't used (specifically, `EmbeddedEventUsed()` will be false).\r\n- Add a check in `IsEventSelected()` in `AliAnalysisTaskEmcal` which (if enabled), will reject events for which `EmbeddedEventUsed()` is false. This check will also need to be added to `AliEmcalCorrectionTask`.\r\n    - The user can also handle this on their own if they are so inclined.\r\n\r\nOne of the main reasons that we propose this approach is in that we are rather concerned about matching event selection in every task. Since AliEventCuts is not implemented in `AliAnalysisTaskEmcal`, matching that to other user tasks could be difficult. In our case, everything follows the lead of the selection in the embedding helper.\r\n\r\nI still see the appeal of the simplicity of your approach, but I see no way around the issue of NextEvent() being called before IsEventSelected() in `AliAnalysisTaskEmcal`, which means that we call `ExternalEvent()` before we apply event selection. But I'm happy to hear of alternatives.\r\n\r\nI think I'll probably start implementing this, but comments are still definitely appreciated!\r\n\r\nBest,\r\nRaymond"}}
{"actor":{"display_login":"raymondEhlers"}}