{"action":"created","issue":{"url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/121","repository_url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror","labels_url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/121/labels{/name}","comments_url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/121/comments","events_url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/121/events","html_url":"https://github.com/spaulaus/paass-laughing-conqueror/pull/121","id":300094600,"number":121,"title":"Adds native ROOT histogramming to utkscan","user":{"login":"spaulaus","id":1553042,"avatar_url":"https://avatars1.githubusercontent.com/u/1553042?v=4","gravatar_id":"","url":"https://api.github.com/users/spaulaus","html_url":"https://github.com/spaulaus","followers_url":"https://api.github.com/users/spaulaus/followers","following_url":"https://api.github.com/users/spaulaus/following{/other_user}","gists_url":"https://api.github.com/users/spaulaus/gists{/gist_id}","starred_url":"https://api.github.com/users/spaulaus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spaulaus/subscriptions","organizations_url":"https://api.github.com/users/spaulaus/orgs","repos_url":"https://api.github.com/users/spaulaus/repos","events_url":"https://api.github.com/users/spaulaus/events{/privacy}","received_events_url":"https://api.github.com/users/spaulaus/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-02-26T03:01:20Z","updated_at":"2018-04-20T11:04:18Z","closed_at":"2018-02-28T01:39:37Z","author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/pulls/121","html_url":"https://github.com/spaulaus/paass-laughing-conqueror/pull/121","diff_url":"https://github.com/spaulaus/paass-laughing-conqueror/pull/121.diff","patch_url":"https://github.com/spaulaus/paass-laughing-conqueror/pull/121.patch"},"body":"<!--- Provide a general summary of your changes in the Title above -->\r\n## Description\r\n<!--- Describe your changes in detail -->\r\n### General remarks\r\nWhooo!!! This marks a major milestone in PAASS. We not have a native ROOT Handler. At the moment this interface is only useful for plotting. I used the old `RootProcessor` as a base, so we have some of the basic tree/branch code still hanging out. The reason I dove into this is because the custom DAMM histogramming killed the scan efficiency. The scanning efficiency is now on the same level as pixie_ldf_c. I started at over 20 minutes to scan a file; I'm down to 45 seconds. Plotting to both DAMM and ROOT in utkscanor doesn't change this value much. \r\n\r\n### Details on RootHandler\r\nThe `RootHandler` is a singleton class. All classes in utkscan can access the same TFile, TTrees, and histograms. This is a marked advantage over the old DAMM histogramming. I wrote the class to accept integers for the histogram IDs. I wanted to keep it in line with DAMM conventions. I reduced the amount of rewrites necessary. The ROOT file contains all of the histograms defined by the software and the DAMM IDs are prepended with an `h`. \r\n\r\n### Updates to Plot class\r\nThe plots class acts as a wrapper for `RootHandler` and made rewriting easier. I wrapped DAMM calls in preprocessor defines to ensured that utkscanor works as expected.  Utkscanor outputs both DAMM and ROOT histograms now. This is probably going to be our defacto choice for experiments until we sort the online ROOT histogramming. Plots should be cleaned up at some point to remove some of the cruft, but it works as expected for now. \r\n\r\n### Updates to the build\r\nUtkscan requires ROOT to build. I've updated the CMake lists accordingly. They only display the utkscan option if `ROOT_FOUND`. If ROOT isn't found, we print a warning telling the user why utkscan is now unavailable. \r\n\r\nI removed all traces of the `useroot` and `USE_ROOT` preprocessor flags from utkscan. Now that we require ROOT these flags are unnecessary. I updated `.travis.yml` to reflect the new dependencies. \r\n\r\n### Updates to Analyzers and Processors\r\nNow that everybody has access to a single instance of plotting, I could dry out the Analyzers and Processors. `DetectorDriver`, `TraceAnalyzer` and `EventProcessor` had identical methods to wrap calls to `Plot` methods. I never liked this, and am happy to see it go. DRYing out these calls caused the line mod counts to spike. \r\n\r\nThese changes occurred in commit 5939480 and 21fa332. I didn't change anything other than Analyzers and Processors. I hope that it makes the code review easier. \r\n\r\n### Important notes\r\n1. utkscan cannot view online histograms since they're now ROOT based. We'll need to figure out how to handle that. \r\n2. utkscan no longer has the ability to write a DAMM file. That's not to imply that it can't write damn files. \r\n\r\n## Related Issue\r\n<!--- This project only accepts pull requests related to open issues -->\r\n<!--- If suggesting a new feature or change, please discuss it in an issue first -->\r\n<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->\r\n<!--- Please link to the issue here: -->\r\n#119 \r\n#120 \r\n[pixie16/paass#7](https://github.com/pixie16/paass/issues/7)\r\n\r\n## Motivation and Context\r\n<!--- Why is this change required? What problem does it solve? -->\r\nThe analysis took far too long to analyze and histogram data. For the same data file, utkscan took over 20 minutes to scan and pixie_ldf_c took 45 seconds. In Issue #119, I dug in and found that the cause was `HisFile` and its associated classes. The file **worked** it just didn't work efficiently. \r\n\r\n## How Has This Been Tested?\r\n<!--- Please describe in detail how you tested your changes. -->\r\n<!--- Include details of your testing environment, and the tests you ran to -->\r\n<!--- see how your change affects other areas of the code, etc. -->\r\nThese changes were contained to utkscan and utkscanor. I didn't modify any of the other libraries or sections of the software. That said, I did full builds using the CI server and dev environment to ensure that nothing else broke. On local, I tested data files for efficiency and results. \r\n\r\n## Screenshots (if appropriate):\r\n### 1D Spectra\r\n![image](https://user-images.githubusercontent.com/1553042/36668502-89c73290-1abf-11e8-8d54-ed44619c0c7b.png)\r\n### Histogram List and 2D spectrum\r\n![image](https://user-images.githubusercontent.com/1553042/36668626-e0795b5e-1abf-11e8-9ae7-43fbfd38280f.png)\r\n\r\n## Types of changes\r\n<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->\r\n- [ ] Bug fix (non-breaking change which fixes an issue)\r\n- [ ] New feature (non-breaking change which adds functionality)\r\n- [X] Breaking change (fix or feature that would cause existing functionality to change)\r\n\r\n## Checklist:\r\n<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->\r\n<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->\r\n- [X] My code follows the code style of this project.\r\n- [X] My change requires a change to the documentation.\r\n- [x] I have updated the documentation accordingly.\r\n- [x] I have read the [Contributing](https://github.com/spaulaus/paass-laughing-conqueror/blob/master/.github/CONTRIBUTING.md) document.\r\n- [x] I have added tests to cover my changes.\r\n- [x] All new and existing tests passed.\r\n"},"comment":{"url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/comments/383062763","html_url":"https://github.com/spaulaus/paass-laughing-conqueror/pull/121#issuecomment-383062763","issue_url":"https://api.github.com/repos/spaulaus/paass-laughing-conqueror/issues/121","id":383062763,"user":{"login":"spaulaus","id":1553042,"avatar_url":"https://avatars1.githubusercontent.com/u/1553042?v=4","gravatar_id":"","url":"https://api.github.com/users/spaulaus","html_url":"https://github.com/spaulaus","followers_url":"https://api.github.com/users/spaulaus/followers","following_url":"https://api.github.com/users/spaulaus/following{/other_user}","gists_url":"https://api.github.com/users/spaulaus/gists{/gist_id}","starred_url":"https://api.github.com/users/spaulaus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/spaulaus/subscriptions","organizations_url":"https://api.github.com/users/spaulaus/orgs","repos_url":"https://api.github.com/users/spaulaus/repos","events_url":"https://api.github.com/users/spaulaus/events{/privacy}","received_events_url":"https://api.github.com/users/spaulaus/received_events","type":"User","site_admin":false},"created_at":"2018-04-20T11:04:18Z","updated_at":"2018-04-20T11:04:18Z","author_association":"OWNER","body":"This issue was moved to spaulaus/paass#121"}}
{"actor":{"display_login":"spaulaus"}}