{"action":"created","comment":{"url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/comments/50573574","id":50573574,"diff_hunk":"@@ -308,19 +307,19 @@ namespace PlantLoopEquip {\n \t\t//Pipe has no special types at the moment, so find it this way\n \t\tif ( GeneralEquipType == GenEquipTypes_Pipe ) {\n \t\t\tif ( EquipTypeNum == TypeOf_Pipe ) {","path":"src/EnergyPlus/PlantLoopEquip.cc","position":20,"original_position":20,"commit_id":"16d63459525c34c924d762b95e712a85285d67a4","original_commit_id":"16d63459525c34c924d762b95e712a85285d67a4","user":{"login":"jasondegraw","id":4623433,"avatar_url":"https://avatars.githubusercontent.com/u/4623433?v=3","gravatar_id":"","url":"https://api.github.com/users/jasondegraw","html_url":"https://github.com/jasondegraw","followers_url":"https://api.github.com/users/jasondegraw/followers","following_url":"https://api.github.com/users/jasondegraw/following{/other_user}","gists_url":"https://api.github.com/users/jasondegraw/gists{/gist_id}","starred_url":"https://api.github.com/users/jasondegraw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasondegraw/subscriptions","organizations_url":"https://api.github.com/users/jasondegraw/orgs","repos_url":"https://api.github.com/users/jasondegraw/repos","events_url":"https://api.github.com/users/jasondegraw/events{/privacy}","received_events_url":"https://api.github.com/users/jasondegraw/received_events","type":"User","site_admin":false},"body":"Maybe collapsing all of the types that use `->simulate` into a single `if` clause would be \"better\":\r\n\r\n    if ( EquipTypeNum == TypeOf_Pipe  || EquipTypeNum == TypeOf_PipeSteam || EquipTypeNum == TypeOf_PipeExterior ...) {","created_at":"2016-01-22T18:41:19Z","updated_at":"2016-01-22T18:41:19Z","html_url":"https://github.com/NREL/EnergyPlus/pull/5426#discussion_r50573574","pull_request_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426","_links":{"self":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/comments/50573574"},"html":{"href":"https://github.com/NREL/EnergyPlus/pull/5426#discussion_r50573574"},"pull_request":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426"}}},"pull_request":{"url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426","id":56494093,"html_url":"https://github.com/NREL/EnergyPlus/pull/5426","diff_url":"https://github.com/NREL/EnergyPlus/pull/5426.diff","patch_url":"https://github.com/NREL/EnergyPlus/pull/5426.patch","issue_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/5426","number":5426,"state":"open","locked":false,"title":"Implement a virtual base class for plant component simulation","user":{"login":"Myoldmopar","id":849824,"avatar_url":"https://avatars.githubusercontent.com/u/849824?v=3","gravatar_id":"","url":"https://api.github.com/users/Myoldmopar","html_url":"https://github.com/Myoldmopar","followers_url":"https://api.github.com/users/Myoldmopar/followers","following_url":"https://api.github.com/users/Myoldmopar/following{/other_user}","gists_url":"https://api.github.com/users/Myoldmopar/gists{/gist_id}","starred_url":"https://api.github.com/users/Myoldmopar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Myoldmopar/subscriptions","organizations_url":"https://api.github.com/users/Myoldmopar/orgs","repos_url":"https://api.github.com/users/Myoldmopar/repos","events_url":"https://api.github.com/users/Myoldmopar/events{/privacy}","received_events_url":"https://api.github.com/users/Myoldmopar/received_events","type":"User","site_admin":false},"body":"Yay\r\n===\r\n\r\nRestart the plant base class work again.  This time in small steps to see when any diffs are introduced.\r\n\r\n### Fluidity\r\n\r\n - This is **very** fluid right now, so don't base anything on this.  \r\n\r\n### Phase 0\r\n\r\n - Create a PlantComponent base class for `simulate()` only.\r\n - Add a shared pointer on the plant data structure for later calling `simulate()` on an instance.\r\n\r\n### Phase 1: Virtualize `SimXYZ()`\r\n\r\nFor each component to change, the following must be done:\r\n\r\n - Make the component struct inherit the PlantComponent virtual base class\r\n - Modify the component storage so that it is an array of shared pointers to objects, not objects themselves\r\n - Create a static `factory` function in the component struct that verifies input has been \"gotten\" for that class of components, and returns the shared pointer (stored in the namespace array) to the component instance matching type/name, or executes a fatal error.\r\n - Create a `simulate()` member function on the struct that either replaces the original `SimXYZ` namespace function, or wraps it nicely.\r\n - Modify the `PlantManager` input to call the factory method for that particular type and assign the shared pointer on the plant structure.\r\n - Modify the `PlantLoopEquip` file to call the member function instead of the `SimXYZ` function.\r\n - Verify no-diffs or very explainable diffs.  But probably none.\r\n\r\n### Phase 2\r\n\r\nBegin virtualizing other parts of the components.  Likely possibilities are:\r\n\r\n - `oneTimeInit()`\r\n - `firstHVACInit()`\r\n\r\nOnce these are propagated through the entire set of plant components, I strongly expect a nice performance boost because we will eliminate so many checks littered heavily throughout the Sim and Init functions.\r\n\r\n### Which Components?\r\n\r\nThe design must be exercised through a variety of component types.  The following list should suffice:\r\n\r\n - [ ] Regular pipes (one inlet/outlet pair, very simple)\r\n - [ ] Heat transfer pipes (same topology as regular pipes, but added complexity)\r\n - [ ] Const COP chiller (up to 3 plant connections, lots of complexity)\r\n - [ ] Pumps (one inlet/outlet, but different calling structure from other plant components)\r\n\r\n### Tips during refactoring\r\n\r\n - Watch for ambiguity with struct member variables and module level variables.  This is a huge possibility for a source of diffs and confusion.  Best approach: remove all module level variables first, or at least rename them something different from the struct member variable.  Once this version compiles fine, that should be much less of an issue moving forward.\r\n - Analyze all static (non-member functions: Basically, those functions that don't have a CompIndex type of argument) to see if the functionality can be split out and called separately from the member function operations.\r\n - Could consider doing all the member function work first to verify the worker functions can be objectified easily, then do Sim() after that.\r\n\r\n### My To Do\r\n\r\n - Go back and evaluate encapsulation, less public members and functions\r\n - Go back and eliminate namespace level variables","created_at":"2016-01-19T16:57:46Z","updated_at":"2016-01-22T18:41:19Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1ad55de90816af17a9500e2db85f5112170b725a","assignee":{"login":"Myoldmopar","id":849824,"avatar_url":"https://avatars.githubusercontent.com/u/849824?v=3","gravatar_id":"","url":"https://api.github.com/users/Myoldmopar","html_url":"https://github.com/Myoldmopar","followers_url":"https://api.github.com/users/Myoldmopar/followers","following_url":"https://api.github.com/users/Myoldmopar/following{/other_user}","gists_url":"https://api.github.com/users/Myoldmopar/gists{/gist_id}","starred_url":"https://api.github.com/users/Myoldmopar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Myoldmopar/subscriptions","organizations_url":"https://api.github.com/users/Myoldmopar/orgs","repos_url":"https://api.github.com/users/Myoldmopar/repos","events_url":"https://api.github.com/users/Myoldmopar/events{/privacy}","received_events_url":"https://api.github.com/users/Myoldmopar/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426/commits","review_comments_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426/comments","review_comment_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls/comments{/number}","comments_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/5426/comments","statuses_url":"https://api.github.com/repos/NREL/EnergyPlus/statuses/16d63459525c34c924d762b95e712a85285d67a4","head":{"label":"NREL:PlantCompBaseClassAgain","ref":"PlantCompBaseClassAgain","sha":"16d63459525c34c924d762b95e712a85285d67a4","user":{"login":"NREL","id":1906800,"avatar_url":"https://avatars.githubusercontent.com/u/1906800?v=3","gravatar_id":"","url":"https://api.github.com/users/NREL","html_url":"https://github.com/NREL","followers_url":"https://api.github.com/users/NREL/followers","following_url":"https://api.github.com/users/NREL/following{/other_user}","gists_url":"https://api.github.com/users/NREL/gists{/gist_id}","starred_url":"https://api.github.com/users/NREL/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NREL/subscriptions","organizations_url":"https://api.github.com/users/NREL/orgs","repos_url":"https://api.github.com/users/NREL/repos","events_url":"https://api.github.com/users/NREL/events{/privacy}","received_events_url":"https://api.github.com/users/NREL/received_events","type":"Organization","site_admin":false},"repo":{"id":14620185,"name":"EnergyPlus","full_name":"NREL/EnergyPlus","owner":{"login":"NREL","id":1906800,"avatar_url":"https://avatars.githubusercontent.com/u/1906800?v=3","gravatar_id":"","url":"https://api.github.com/users/NREL","html_url":"https://github.com/NREL","followers_url":"https://api.github.com/users/NREL/followers","following_url":"https://api.github.com/users/NREL/following{/other_user}","gists_url":"https://api.github.com/users/NREL/gists{/gist_id}","starred_url":"https://api.github.com/users/NREL/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NREL/subscriptions","organizations_url":"https://api.github.com/users/NREL/orgs","repos_url":"https://api.github.com/users/NREL/repos","events_url":"https://api.github.com/users/NREL/events{/privacy}","received_events_url":"https://api.github.com/users/NREL/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/NREL/EnergyPlus","description":"EnergyPlus™ is a whole building energy simulation program that engineers, architects, and researchers use to model both energy consumption and water use in buildings.","fork":false,"url":"https://api.github.com/repos/NREL/EnergyPlus","forks_url":"https://api.github.com/repos/NREL/EnergyPlus/forks","keys_url":"https://api.github.com/repos/NREL/EnergyPlus/keys{/key_id}","collaborators_url":"https://api.github.com/repos/NREL/EnergyPlus/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/NREL/EnergyPlus/teams","hooks_url":"https://api.github.com/repos/NREL/EnergyPlus/hooks","issue_events_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/events{/number}","events_url":"https://api.github.com/repos/NREL/EnergyPlus/events","assignees_url":"https://api.github.com/repos/NREL/EnergyPlus/assignees{/user}","branches_url":"https://api.github.com/repos/NREL/EnergyPlus/branches{/branch}","tags_url":"https://api.github.com/repos/NREL/EnergyPlus/tags","blobs_url":"https://api.github.com/repos/NREL/EnergyPlus/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/NREL/EnergyPlus/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/NREL/EnergyPlus/git/refs{/sha}","trees_url":"https://api.github.com/repos/NREL/EnergyPlus/git/trees{/sha}","statuses_url":"https://api.github.com/repos/NREL/EnergyPlus/statuses/{sha}","languages_url":"https://api.github.com/repos/NREL/EnergyPlus/languages","stargazers_url":"https://api.github.com/repos/NREL/EnergyPlus/stargazers","contributors_url":"https://api.github.com/repos/NREL/EnergyPlus/contributors","subscribers_url":"https://api.github.com/repos/NREL/EnergyPlus/subscribers","subscription_url":"https://api.github.com/repos/NREL/EnergyPlus/subscription","commits_url":"https://api.github.com/repos/NREL/EnergyPlus/commits{/sha}","git_commits_url":"https://api.github.com/repos/NREL/EnergyPlus/git/commits{/sha}","comments_url":"https://api.github.com/repos/NREL/EnergyPlus/comments{/number}","issue_comment_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/comments{/number}","contents_url":"https://api.github.com/repos/NREL/EnergyPlus/contents/{+path}","compare_url":"https://api.github.com/repos/NREL/EnergyPlus/compare/{base}...{head}","merges_url":"https://api.github.com/repos/NREL/EnergyPlus/merges","archive_url":"https://api.github.com/repos/NREL/EnergyPlus/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/NREL/EnergyPlus/downloads","issues_url":"https://api.github.com/repos/NREL/EnergyPlus/issues{/number}","pulls_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls{/number}","milestones_url":"https://api.github.com/repos/NREL/EnergyPlus/milestones{/number}","notifications_url":"https://api.github.com/repos/NREL/EnergyPlus/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/NREL/EnergyPlus/labels{/name}","releases_url":"https://api.github.com/repos/NREL/EnergyPlus/releases{/id}","deployments_url":"https://api.github.com/repos/NREL/EnergyPlus/deployments","created_at":"2013-11-22T14:47:34Z","updated_at":"2016-01-21T08:12:00Z","pushed_at":"2016-01-22T17:03:39Z","git_url":"git://github.com/NREL/EnergyPlus.git","ssh_url":"git@github.com:NREL/EnergyPlus.git","clone_url":"https://github.com/NREL/EnergyPlus.git","svn_url":"https://github.com/NREL/EnergyPlus","homepage":"https://energyplus.net","size":229367,"stargazers_count":83,"watchers_count":83,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":56,"mirror_url":null,"open_issues_count":433,"forks":56,"open_issues":433,"watchers":83,"default_branch":"develop"}},"base":{"label":"NREL:develop","ref":"develop","sha":"d8a1ba73c27c0e3cef866ba8eebcadb00ef9ade8","user":{"login":"NREL","id":1906800,"avatar_url":"https://avatars.githubusercontent.com/u/1906800?v=3","gravatar_id":"","url":"https://api.github.com/users/NREL","html_url":"https://github.com/NREL","followers_url":"https://api.github.com/users/NREL/followers","following_url":"https://api.github.com/users/NREL/following{/other_user}","gists_url":"https://api.github.com/users/NREL/gists{/gist_id}","starred_url":"https://api.github.com/users/NREL/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NREL/subscriptions","organizations_url":"https://api.github.com/users/NREL/orgs","repos_url":"https://api.github.com/users/NREL/repos","events_url":"https://api.github.com/users/NREL/events{/privacy}","received_events_url":"https://api.github.com/users/NREL/received_events","type":"Organization","site_admin":false},"repo":{"id":14620185,"name":"EnergyPlus","full_name":"NREL/EnergyPlus","owner":{"login":"NREL","id":1906800,"avatar_url":"https://avatars.githubusercontent.com/u/1906800?v=3","gravatar_id":"","url":"https://api.github.com/users/NREL","html_url":"https://github.com/NREL","followers_url":"https://api.github.com/users/NREL/followers","following_url":"https://api.github.com/users/NREL/following{/other_user}","gists_url":"https://api.github.com/users/NREL/gists{/gist_id}","starred_url":"https://api.github.com/users/NREL/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NREL/subscriptions","organizations_url":"https://api.github.com/users/NREL/orgs","repos_url":"https://api.github.com/users/NREL/repos","events_url":"https://api.github.com/users/NREL/events{/privacy}","received_events_url":"https://api.github.com/users/NREL/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/NREL/EnergyPlus","description":"EnergyPlus™ is a whole building energy simulation program that engineers, architects, and researchers use to model both energy consumption and water use in buildings.","fork":false,"url":"https://api.github.com/repos/NREL/EnergyPlus","forks_url":"https://api.github.com/repos/NREL/EnergyPlus/forks","keys_url":"https://api.github.com/repos/NREL/EnergyPlus/keys{/key_id}","collaborators_url":"https://api.github.com/repos/NREL/EnergyPlus/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/NREL/EnergyPlus/teams","hooks_url":"https://api.github.com/repos/NREL/EnergyPlus/hooks","issue_events_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/events{/number}","events_url":"https://api.github.com/repos/NREL/EnergyPlus/events","assignees_url":"https://api.github.com/repos/NREL/EnergyPlus/assignees{/user}","branches_url":"https://api.github.com/repos/NREL/EnergyPlus/branches{/branch}","tags_url":"https://api.github.com/repos/NREL/EnergyPlus/tags","blobs_url":"https://api.github.com/repos/NREL/EnergyPlus/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/NREL/EnergyPlus/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/NREL/EnergyPlus/git/refs{/sha}","trees_url":"https://api.github.com/repos/NREL/EnergyPlus/git/trees{/sha}","statuses_url":"https://api.github.com/repos/NREL/EnergyPlus/statuses/{sha}","languages_url":"https://api.github.com/repos/NREL/EnergyPlus/languages","stargazers_url":"https://api.github.com/repos/NREL/EnergyPlus/stargazers","contributors_url":"https://api.github.com/repos/NREL/EnergyPlus/contributors","subscribers_url":"https://api.github.com/repos/NREL/EnergyPlus/subscribers","subscription_url":"https://api.github.com/repos/NREL/EnergyPlus/subscription","commits_url":"https://api.github.com/repos/NREL/EnergyPlus/commits{/sha}","git_commits_url":"https://api.github.com/repos/NREL/EnergyPlus/git/commits{/sha}","comments_url":"https://api.github.com/repos/NREL/EnergyPlus/comments{/number}","issue_comment_url":"https://api.github.com/repos/NREL/EnergyPlus/issues/comments{/number}","contents_url":"https://api.github.com/repos/NREL/EnergyPlus/contents/{+path}","compare_url":"https://api.github.com/repos/NREL/EnergyPlus/compare/{base}...{head}","merges_url":"https://api.github.com/repos/NREL/EnergyPlus/merges","archive_url":"https://api.github.com/repos/NREL/EnergyPlus/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/NREL/EnergyPlus/downloads","issues_url":"https://api.github.com/repos/NREL/EnergyPlus/issues{/number}","pulls_url":"https://api.github.com/repos/NREL/EnergyPlus/pulls{/number}","milestones_url":"https://api.github.com/repos/NREL/EnergyPlus/milestones{/number}","notifications_url":"https://api.github.com/repos/NREL/EnergyPlus/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/NREL/EnergyPlus/labels{/name}","releases_url":"https://api.github.com/repos/NREL/EnergyPlus/releases{/id}","deployments_url":"https://api.github.com/repos/NREL/EnergyPlus/deployments","created_at":"2013-11-22T14:47:34Z","updated_at":"2016-01-21T08:12:00Z","pushed_at":"2016-01-22T17:03:39Z","git_url":"git://github.com/NREL/EnergyPlus.git","ssh_url":"git@github.com:NREL/EnergyPlus.git","clone_url":"https://github.com/NREL/EnergyPlus.git","svn_url":"https://github.com/NREL/EnergyPlus","homepage":"https://energyplus.net","size":229367,"stargazers_count":83,"watchers_count":83,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":56,"mirror_url":null,"open_issues_count":433,"forks":56,"open_issues":433,"watchers":83,"default_branch":"develop"}},"_links":{"self":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426"},"html":{"href":"https://github.com/NREL/EnergyPlus/pull/5426"},"issue":{"href":"https://api.github.com/repos/NREL/EnergyPlus/issues/5426"},"comments":{"href":"https://api.github.com/repos/NREL/EnergyPlus/issues/5426/comments"},"review_comments":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426/comments"},"review_comment":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/NREL/EnergyPlus/pulls/5426/commits"},"statuses":{"href":"https://api.github.com/repos/NREL/EnergyPlus/statuses/16d63459525c34c924d762b95e712a85285d67a4"}}}}