{"action":"created","comment":{"url":"https://api.github.com/repos/mantidproject/documents/pulls/comments/58375879","id":58375879,"diff_hunk":"@@ -0,0 +1,108 @@\n+# Histogram Data Type\n+\n+**Note:** This document is mainly concerned with a data type for a *generic* histogram.\n+In the context considered here, a histogram is *not* specific to neutron science and probably not even specific to physics.\n+In particular it should not be confused with the current data types in Mantid that are joint containers for histogram data and spectrum/detector information, such as `ISpectrum` and `Histogram1D`.\n+\n+\n+## Motivation\n+\n+One of the most fundamental concepts of ubiquitous use in Mantid is that of histograms of (event) data.\n+Despite this, there is currently no data type in Mantid that deals with histogram data.\n+The purpose of this design proposal is to fix this.\n+\n+There are various motivations for this:\n+\n+- Reduce code duplication by providing arithmetic operations such as addition of histograms.\n+- Hide handling of errors (uncertainties).\n+- Hide discrimination between histogram and distribution data.\n+- Decrease risk for certain types of bugs by using the C++ type system.\n+\n+All this would remove duplicated code from the client and put it into a central place where it can be unit-tested.\n+\n+\n+## Current situation\n+\n+The current way of dealing with histograms has various shortcomings.\n+Basically we have a set of three `std::vector<double>`, for `X`, `Y`, and `E`.\n+There is a single `typedef` `MantidVec` used for all three of them (and other data), and this is probably adding only more confusion.\n+\n+The problems include:\n+\n+- No type safety:\n+  - The bug in `ConvertUnits` creped in because we are currently using the same instance of a single type for both histogram data and distribution data. The actual meaning of a vector of `Y` values is basically kept as a mental flag in the brain of a Mantid developer.\n+  - We are using a single type for `X`, `Y`, and `E`. It is thus easy to accidentally mix them up.\n+  - The array `E` of error values is used for both the variance and the standard deviation. The state is again carried as a flag in the brain of a developer. We even have functions such as `rebin` that return an array of `E` values that may mean either variance or standard deviation depending on a flag to the function!\n+- Code duplication: Re-implementing the same thing 10 times is wasteful and error prone.\n+  - Operations like adding histograms are frequent.\n+  - Propagation of errors.\n+  - Conversions between variance and standard deviation.\n+- Unit testing: Implementation of operations on histogram are currently mixed into algorithm code. Unit tests thus need to test two things at the same time.\n+  - Many units tests of algorithms do not fully test the range of errors that can happen when, e.g., adding two spectra. Doing so would be very hard in the current de-centralized implementation,\n+  - Many algorithms need to modify the errors. In some algorithms the unit tests cover the errors, in others they do not.\n+\n+\n+## Design goals\n+\n+- Provide simple arithmetic operations for histograms.\n+  - Arithmetic operations automatically deal with propagation of errors.\n+- Hide differences between histogram and distribution data.\n+  - Internal storage type could be either, does not need to be known at the interface level?\n+  - Arithmetic operations should do the right thing for any data type automatically.\n+- Hide error handling.\n+  - For simple operations with histograms algorithm authors should not need to know how to propagate errors.\n+- Enable unit testing of key concepts in Mantid.\n+  - Arithmetic operations with histograms.\n+  - Propagation of errors.\n+\n+\n+\n+## Implementation\n+\n+#### Basics\n+\n+The details of the implementation have not been worked out yet.\n+The basic concept seems simple enough:\n+\n+- Add one class for a histogram with an interface that provides standard operations for histogram data.\n+  - Note that we do not introduce a separate type for histogram and distribution data. This distinction would be only internal, and access to either histogram or distribution data is provided via public interface methods. The goal is to eliminate this distinction from as much of the code as possible.\n+- Add a few classes for `X`, `Y`, and `E` (etc.) that would mainly be used internally in the histogram class and for direct access to the underlying data. If these inherit from `std::vetor<double>`, they could also be used to keep the legacy interface for `ISpectrum` alive until the roll-out is complete.\n+\n+#### Details\n+\n+- It is probably useful to introduce two types for vectors of standard deviation and variance, instead of the current double use of `E`.\n+- Similarly we should have distinct internal types for histogram and distribution data, in case our implementation relies on this distinction. Jon pointed out that it may make most sense to default to distribution data, as OpenGenie did by default.","path":"Design/histogram_type.md","position":74,"original_position":74,"commit_id":"007e20b7ae05ad2134c35a379c1db70c63e411f2","original_commit_id":"007e20b7ae05ad2134c35a379c1db70c63e411f2","user":{"login":"NickDraper","id":1017173,"avatar_url":"https://avatars.githubusercontent.com/u/1017173?v=3","gravatar_id":"","url":"https://api.github.com/users/NickDraper","html_url":"https://github.com/NickDraper","followers_url":"https://api.github.com/users/NickDraper/followers","following_url":"https://api.github.com/users/NickDraper/following{/other_user}","gists_url":"https://api.github.com/users/NickDraper/gists{/gist_id}","starred_url":"https://api.github.com/users/NickDraper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NickDraper/subscriptions","organizations_url":"https://api.github.com/users/NickDraper/orgs","repos_url":"https://api.github.com/users/NickDraper/repos","events_url":"https://api.github.com/users/NickDraper/events{/privacy}","received_events_url":"https://api.github.com/users/NickDraper/received_events","type":"User","site_admin":false},"body":"I'm a little concerned about specifying id the data is a distribution or histogram at this level.  While I agree that it makes sense computationally, I cannot come up with a valid use case where you would have a mixture within a single workspace.  Taking this approach would make that possible, and if it is possible within the code someone somewhere will do it, and I suspect that very few algorithms would handle that situation gracefully (and they shouldn't have to).  The current approach of having the distribution flag on the workspace has it's flaws, but that is at the level it makes sense to define it for a workspace, either all distribution or none.","created_at":"2016-04-04T13:36:41Z","updated_at":"2016-04-04T13:36:41Z","html_url":"https://github.com/mantidproject/documents/pull/14#discussion_r58375879","pull_request_url":"https://api.github.com/repos/mantidproject/documents/pulls/14","_links":{"self":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/comments/58375879"},"html":{"href":"https://github.com/mantidproject/documents/pull/14#discussion_r58375879"},"pull_request":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/14"}}},"pull_request":{"url":"https://api.github.com/repos/mantidproject/documents/pulls/14","id":62905559,"html_url":"https://github.com/mantidproject/documents/pull/14","diff_url":"https://github.com/mantidproject/documents/pull/14.diff","patch_url":"https://github.com/mantidproject/documents/pull/14.patch","issue_url":"https://api.github.com/repos/mantidproject/documents/issues/14","number":14,"state":"open","locked":false,"title":"Histogram type","user":{"login":"SimonHeybrock","id":12912489,"avatar_url":"https://avatars.githubusercontent.com/u/12912489?v=3","gravatar_id":"","url":"https://api.github.com/users/SimonHeybrock","html_url":"https://github.com/SimonHeybrock","followers_url":"https://api.github.com/users/SimonHeybrock/followers","following_url":"https://api.github.com/users/SimonHeybrock/following{/other_user}","gists_url":"https://api.github.com/users/SimonHeybrock/gists{/gist_id}","starred_url":"https://api.github.com/users/SimonHeybrock/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SimonHeybrock/subscriptions","organizations_url":"https://api.github.com/users/SimonHeybrock/orgs","repos_url":"https://api.github.com/users/SimonHeybrock/repos","events_url":"https://api.github.com/users/SimonHeybrock/events{/privacy}","received_events_url":"https://api.github.com/users/SimonHeybrock/received_events","type":"User","site_admin":false},"body":"First draft of a design proposal for a histogram data type in Mantid. Please comment!","created_at":"2016-03-15T13:33:08Z","updated_at":"2016-04-04T13:36:41Z","closed_at":null,"merged_at":null,"merge_commit_sha":"4dc68569392f9a31cbb0cb860ad72a71e0a2348d","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/mantidproject/documents/pulls/14/commits","review_comments_url":"https://api.github.com/repos/mantidproject/documents/pulls/14/comments","review_comment_url":"https://api.github.com/repos/mantidproject/documents/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mantidproject/documents/issues/14/comments","statuses_url":"https://api.github.com/repos/mantidproject/documents/statuses/007e20b7ae05ad2134c35a379c1db70c63e411f2","head":{"label":"mantidproject:histogram_type","ref":"histogram_type","sha":"007e20b7ae05ad2134c35a379c1db70c63e411f2","user":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"repo":{"id":2577235,"name":"documents","full_name":"mantidproject/documents","owner":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mantidproject/documents","description":"Holds documents relating to the Mantid project","fork":false,"url":"https://api.github.com/repos/mantidproject/documents","forks_url":"https://api.github.com/repos/mantidproject/documents/forks","keys_url":"https://api.github.com/repos/mantidproject/documents/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mantidproject/documents/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mantidproject/documents/teams","hooks_url":"https://api.github.com/repos/mantidproject/documents/hooks","issue_events_url":"https://api.github.com/repos/mantidproject/documents/issues/events{/number}","events_url":"https://api.github.com/repos/mantidproject/documents/events","assignees_url":"https://api.github.com/repos/mantidproject/documents/assignees{/user}","branches_url":"https://api.github.com/repos/mantidproject/documents/branches{/branch}","tags_url":"https://api.github.com/repos/mantidproject/documents/tags","blobs_url":"https://api.github.com/repos/mantidproject/documents/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mantidproject/documents/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mantidproject/documents/git/refs{/sha}","trees_url":"https://api.github.com/repos/mantidproject/documents/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mantidproject/documents/statuses/{sha}","languages_url":"https://api.github.com/repos/mantidproject/documents/languages","stargazers_url":"https://api.github.com/repos/mantidproject/documents/stargazers","contributors_url":"https://api.github.com/repos/mantidproject/documents/contributors","subscribers_url":"https://api.github.com/repos/mantidproject/documents/subscribers","subscription_url":"https://api.github.com/repos/mantidproject/documents/subscription","commits_url":"https://api.github.com/repos/mantidproject/documents/commits{/sha}","git_commits_url":"https://api.github.com/repos/mantidproject/documents/git/commits{/sha}","comments_url":"https://api.github.com/repos/mantidproject/documents/comments{/number}","issue_comment_url":"https://api.github.com/repos/mantidproject/documents/issues/comments{/number}","contents_url":"https://api.github.com/repos/mantidproject/documents/contents/{+path}","compare_url":"https://api.github.com/repos/mantidproject/documents/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mantidproject/documents/merges","archive_url":"https://api.github.com/repos/mantidproject/documents/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mantidproject/documents/downloads","issues_url":"https://api.github.com/repos/mantidproject/documents/issues{/number}","pulls_url":"https://api.github.com/repos/mantidproject/documents/pulls{/number}","milestones_url":"https://api.github.com/repos/mantidproject/documents/milestones{/number}","notifications_url":"https://api.github.com/repos/mantidproject/documents/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mantidproject/documents/labels{/name}","releases_url":"https://api.github.com/repos/mantidproject/documents/releases{/id}","deployments_url":"https://api.github.com/repos/mantidproject/documents/deployments","created_at":"2011-10-14T16:05:10Z","updated_at":"2016-01-10T19:15:15Z","pushed_at":"2016-04-04T13:35:21Z","git_url":"git://github.com/mantidproject/documents.git","ssh_url":"git@github.com:mantidproject/documents.git","clone_url":"https://github.com/mantidproject/documents.git","svn_url":"https://github.com/mantidproject/documents","homepage":"www.mantidproject.org","size":1287250,"stargazers_count":3,"watchers_count":3,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":11,"forks":2,"open_issues":11,"watchers":3,"default_branch":"master"}},"base":{"label":"mantidproject:master","ref":"master","sha":"b45e29a4b5528e7de17b682278894b8b0be36076","user":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"repo":{"id":2577235,"name":"documents","full_name":"mantidproject/documents","owner":{"login":"mantidproject","id":671496,"avatar_url":"https://avatars.githubusercontent.com/u/671496?v=3","gravatar_id":"","url":"https://api.github.com/users/mantidproject","html_url":"https://github.com/mantidproject","followers_url":"https://api.github.com/users/mantidproject/followers","following_url":"https://api.github.com/users/mantidproject/following{/other_user}","gists_url":"https://api.github.com/users/mantidproject/gists{/gist_id}","starred_url":"https://api.github.com/users/mantidproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mantidproject/subscriptions","organizations_url":"https://api.github.com/users/mantidproject/orgs","repos_url":"https://api.github.com/users/mantidproject/repos","events_url":"https://api.github.com/users/mantidproject/events{/privacy}","received_events_url":"https://api.github.com/users/mantidproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mantidproject/documents","description":"Holds documents relating to the Mantid project","fork":false,"url":"https://api.github.com/repos/mantidproject/documents","forks_url":"https://api.github.com/repos/mantidproject/documents/forks","keys_url":"https://api.github.com/repos/mantidproject/documents/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mantidproject/documents/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mantidproject/documents/teams","hooks_url":"https://api.github.com/repos/mantidproject/documents/hooks","issue_events_url":"https://api.github.com/repos/mantidproject/documents/issues/events{/number}","events_url":"https://api.github.com/repos/mantidproject/documents/events","assignees_url":"https://api.github.com/repos/mantidproject/documents/assignees{/user}","branches_url":"https://api.github.com/repos/mantidproject/documents/branches{/branch}","tags_url":"https://api.github.com/repos/mantidproject/documents/tags","blobs_url":"https://api.github.com/repos/mantidproject/documents/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mantidproject/documents/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mantidproject/documents/git/refs{/sha}","trees_url":"https://api.github.com/repos/mantidproject/documents/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mantidproject/documents/statuses/{sha}","languages_url":"https://api.github.com/repos/mantidproject/documents/languages","stargazers_url":"https://api.github.com/repos/mantidproject/documents/stargazers","contributors_url":"https://api.github.com/repos/mantidproject/documents/contributors","subscribers_url":"https://api.github.com/repos/mantidproject/documents/subscribers","subscription_url":"https://api.github.com/repos/mantidproject/documents/subscription","commits_url":"https://api.github.com/repos/mantidproject/documents/commits{/sha}","git_commits_url":"https://api.github.com/repos/mantidproject/documents/git/commits{/sha}","comments_url":"https://api.github.com/repos/mantidproject/documents/comments{/number}","issue_comment_url":"https://api.github.com/repos/mantidproject/documents/issues/comments{/number}","contents_url":"https://api.github.com/repos/mantidproject/documents/contents/{+path}","compare_url":"https://api.github.com/repos/mantidproject/documents/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mantidproject/documents/merges","archive_url":"https://api.github.com/repos/mantidproject/documents/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mantidproject/documents/downloads","issues_url":"https://api.github.com/repos/mantidproject/documents/issues{/number}","pulls_url":"https://api.github.com/repos/mantidproject/documents/pulls{/number}","milestones_url":"https://api.github.com/repos/mantidproject/documents/milestones{/number}","notifications_url":"https://api.github.com/repos/mantidproject/documents/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mantidproject/documents/labels{/name}","releases_url":"https://api.github.com/repos/mantidproject/documents/releases{/id}","deployments_url":"https://api.github.com/repos/mantidproject/documents/deployments","created_at":"2011-10-14T16:05:10Z","updated_at":"2016-01-10T19:15:15Z","pushed_at":"2016-04-04T13:35:21Z","git_url":"git://github.com/mantidproject/documents.git","ssh_url":"git@github.com:mantidproject/documents.git","clone_url":"https://github.com/mantidproject/documents.git","svn_url":"https://github.com/mantidproject/documents","homepage":"www.mantidproject.org","size":1287250,"stargazers_count":3,"watchers_count":3,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":11,"forks":2,"open_issues":11,"watchers":3,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/14"},"html":{"href":"https://github.com/mantidproject/documents/pull/14"},"issue":{"href":"https://api.github.com/repos/mantidproject/documents/issues/14"},"comments":{"href":"https://api.github.com/repos/mantidproject/documents/issues/14/comments"},"review_comments":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/14/comments"},"review_comment":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mantidproject/documents/pulls/14/commits"},"statuses":{"href":"https://api.github.com/repos/mantidproject/documents/statuses/007e20b7ae05ad2134c35a379c1db70c63e411f2"}}}}