{"action":"created","issue":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/55","repository_url":"https://api.github.com/repos/AtheMathmo/rulinalg","labels_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/55/labels{/name}","comments_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/55/comments","events_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/55/events","html_url":"https://github.com/AtheMathmo/rulinalg/pull/55","id":179407591,"number":55,"title":"Diagonal iterators","user":{"login":"AtheMathmo","id":10150986,"avatar_url":"https://avatars.githubusercontent.com/u/10150986?v=3","gravatar_id":"","url":"https://api.github.com/users/AtheMathmo","html_url":"https://github.com/AtheMathmo","followers_url":"https://api.github.com/users/AtheMathmo/followers","following_url":"https://api.github.com/users/AtheMathmo/following{/other_user}","gists_url":"https://api.github.com/users/AtheMathmo/gists{/gist_id}","starred_url":"https://api.github.com/users/AtheMathmo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AtheMathmo/subscriptions","organizations_url":"https://api.github.com/users/AtheMathmo/orgs","repos_url":"https://api.github.com/users/AtheMathmo/repos","events_url":"https://api.github.com/users/AtheMathmo/events{/privacy}","received_events_url":"https://api.github.com/users/AtheMathmo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":50,"created_at":"2016-09-27T06:30:17Z","updated_at":"2016-10-30T11:01:57Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/pulls/55","html_url":"https://github.com/AtheMathmo/rulinalg/pull/55","diff_url":"https://github.com/AtheMathmo/rulinalg/pull/55.diff","patch_url":"https://github.com/AtheMathmo/rulinalg/pull/55.patch"},"body":"This PR introduces new iterators (`Diagonal`, `DiagonalMut`) for `Matrix`/`MatrixSlice`/`MatrixSliceMut`. It is intended to resolve #53 .\r\n\r\nHere are some benchmarks for collecting the main diagonal of a matrix:\r\n\r\n```\r\ntest linalg::iter::mat_diag_iter_100_500           ... bench:         272 ns/iter (+/- 34)\r\ntest linalg::iter::mat_diag_manual_100_500         ... bench:         309 ns/iter (+/- 87)\r\ntest linalg::iter::mat_diag_iter_10_50             ... bench:          62 ns/iter (+/- 8)\r\ntest linalg::iter::mat_diag_manual_10_50           ... bench:          68 ns/iter (+/- 21)\r\n```\r\n\r\nThe manual version uses unsafe `get_unchecked` indexing _and_ is slower!\r\n\r\nFollowing is kept in place for prosperity but this PR is no longer WIP (pending review).\r\n\r\n---\r\n\r\nThis PR is currently a work in progress and the following should be addressed:\r\n\r\n- How do we handle iteration of off-diagonals?\r\n- How do we expose these iterators?\r\n\r\nI'm thinking we do the following:\r\n\r\nAdd the following:\r\n\r\n```rust\r\npub enum DiagOffset {\r\n    Above(usize),\r\n    Below(usize)\r\n}\r\n```\r\n\r\nThe iterator will then have this enum as a field - which will determine which diagonal offset we use. When creating we will have to check that `Above(x)` has `x < mat.cols()`, and `Below(y)` has `y < mat.rows()`.\r\n\r\nIn terms of exposing the iterator - I think the following is a good approach:\r\n\r\n- Add `fn iter_diag(&self, k: DiagOffset)` to `BaseMatrix`.\r\n- Add `fn iter_diag_mut(&mut self, k: DiagOffset)` to `BaseMatrixMut`.\r\n\r\nWe will keep the `diag` function as returning a `Vec` for now - mostly to prevent breaking changes and we can reassess in future.\r\n\r\nWe will probably need to move the iterators into `matrix/mod` (they are currently in `matrix/iter`). This is so that we can construct them in `matrix/slice`."},"comment":{"url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/comments/257144561","html_url":"https://github.com/AtheMathmo/rulinalg/pull/55#issuecomment-257144561","issue_url":"https://api.github.com/repos/AtheMathmo/rulinalg/issues/55","id":257144561,"user":{"login":"Andlon","id":578309,"avatar_url":"https://avatars.githubusercontent.com/u/578309?v=3","gravatar_id":"","url":"https://api.github.com/users/Andlon","html_url":"https://github.com/Andlon","followers_url":"https://api.github.com/users/Andlon/followers","following_url":"https://api.github.com/users/Andlon/following{/other_user}","gists_url":"https://api.github.com/users/Andlon/gists{/gist_id}","starred_url":"https://api.github.com/users/Andlon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Andlon/subscriptions","organizations_url":"https://api.github.com/users/Andlon/orgs","repos_url":"https://api.github.com/users/Andlon/repos","events_url":"https://api.github.com/users/Andlon/events{/privacy}","received_events_url":"https://api.github.com/users/Andlon/received_events","type":"User","site_admin":false},"created_at":"2016-10-30T11:01:57Z","updated_at":"2016-10-30T11:01:57Z","body":"@AtheMathmo: great! I looked over the latest commit and it looks good to me :)"}}
{"actor":{"display_login":"Andlon"}}