{"action":"created","issue":{"url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/261","repository_url":"https://api.github.com/repos/R3BRootGroup/R3BRoot","labels_url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/261/labels{/name}","comments_url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/261/comments","events_url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/261/events","html_url":"https://github.com/R3BRootGroup/R3BRoot/pull/261","id":474587581,"node_id":"MDExOlB1bGxSZXF1ZXN0MzAyNTAxNzcz","number":261,"title":"R3B distribution","user":{"login":"vadimr3b","id":13536371,"node_id":"MDQ6VXNlcjEzNTM2Mzcx","avatar_url":"https://avatars1.githubusercontent.com/u/13536371?v=4","gravatar_id":"","url":"https://api.github.com/users/vadimr3b","html_url":"https://github.com/vadimr3b","followers_url":"https://api.github.com/users/vadimr3b/followers","following_url":"https://api.github.com/users/vadimr3b/following{/other_user}","gists_url":"https://api.github.com/users/vadimr3b/gists{/gist_id}","starred_url":"https://api.github.com/users/vadimr3b/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vadimr3b/subscriptions","organizations_url":"https://api.github.com/users/vadimr3b/orgs","repos_url":"https://api.github.com/users/vadimr3b/repos","events_url":"https://api.github.com/users/vadimr3b/events{/privacy}","received_events_url":"https://api.github.com/users/vadimr3b/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2019-07-30T13:32:19Z","updated_at":"2019-08-02T12:30:58Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/pulls/261","html_url":"https://github.com/R3BRootGroup/R3BRoot/pull/261","diff_url":"https://github.com/R3BRootGroup/R3BRoot/pull/261.diff","patch_url":"https://github.com/R3BRootGroup/R3BRoot/pull/261.patch"},"body":"I added some abstraction for random number distributions.\r\n\r\nThis should improve the usability, since it removes the necessity to edit the source code to change for example the beam energy distribution (see changes in R3BPhaseSpaceGenerator).\r\n\r\nIn addition this should improve the readability of the source code, since it removes the mathematical and technical details (see changes in R3BCosmicGenerator).\r\n\r\nPlease let me know, if anybody has some suggestions, on how to improve this PR."},"comment":{"url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/comments/517682064","html_url":"https://github.com/R3BRootGroup/R3BRoot/pull/261#issuecomment-517682064","issue_url":"https://api.github.com/repos/R3BRootGroup/R3BRoot/issues/261","id":517682064,"node_id":"MDEyOklzc3VlQ29tbWVudDUxNzY4MjA2NA==","user":{"login":"janmayer","id":4602851,"node_id":"MDQ6VXNlcjQ2MDI4NTE=","avatar_url":"https://avatars3.githubusercontent.com/u/4602851?v=4","gravatar_id":"","url":"https://api.github.com/users/janmayer","html_url":"https://github.com/janmayer","followers_url":"https://api.github.com/users/janmayer/followers","following_url":"https://api.github.com/users/janmayer/following{/other_user}","gists_url":"https://api.github.com/users/janmayer/gists{/gist_id}","starred_url":"https://api.github.com/users/janmayer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/janmayer/subscriptions","organizations_url":"https://api.github.com/users/janmayer/orgs","repos_url":"https://api.github.com/users/janmayer/repos","events_url":"https://api.github.com/users/janmayer/events{/privacy}","received_events_url":"https://api.github.com/users/janmayer/received_events","type":"User","site_admin":false},"created_at":"2019-08-02T12:30:58Z","updated_at":"2019-08-02T12:30:58Z","author_association":"CONTRIBUTOR","body":"Hey, good idea working on the generators. I always felt my sense of order tingle looking at the Cosmic Generator code that someone wrote in Fortran decades ago.\r\n\r\nHere are my thoughts in descending order of importance:\r\n- R3BDouble\r\n    My first reaction was \"Wait what?\", then \"WTF?\", followed by \"Why would you do that?\". One should be able to safe primitive values in the Tree, right? R3BDouble doesn't even derive from TObject. And then using shared pointers with them seems like extraordinary overkill.\r\n- R3BDistribution\r\n    - While I like the general idea and you probably had fun building them, I'm not fully convinced that it was actually necessary. I wonder if there are existing libraries that do most of what you need? Either in `<random>`, somewhere in Boost, or similar (I didn't check)?\r\n    - Why do `fValues` (+`CreateValues`) and `GetValueAddresses` even exist? Super complicated. There is probably a better way to store them in a tree.\r\n    - Why does the user have to supply random values? `Array GetRandomValues(const Array rnd)` That is *exactly* what the class should be handling on its own. No need to bother the user with that.\r\n    - There are no tests for the distribution classes.\r\n    - The distribution classes do not really belong in `r3bgen/` - what about moving them to a new `utility/` folder or similar?\r\n\r\n- General\r\n    - When implementing derived classes, use the `override` keyword, e.g. in `R3BCosmicGenerator`\r\n    - Imho, no test required to build & test R3BRoot should be in the external macro directory\r\n    - The files are not formatted with clang-format\r\n    - Consider deleting unnecessary comments, e.g. the header in `R3BCosmicGenerator.cxx` and the spacer comments (`// ---------`)\r\n\r\nHave to tried to reproduce distributions like in the Sn-Test-Files with the PhaseSpaceGenerator? I had trouble with that and never got to fully fixing it.\r\n"}}
{"actor":{"display_login":"janmayer"}}