{"action":"created","issue":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/712","repository_url":"https://api.github.com/repos/MRtrix3/mrtrix3","labels_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/712/labels{/name}","comments_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/712/comments","events_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/712/events","html_url":"https://github.com/MRtrix3/mrtrix3/pull/712","id":164627357,"number":712,"title":"Port to eigen 3.3","user":{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/labels/enhancement","name":"enhancement","color":"84b6eb"},{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/labels/test%20wanted","name":"test wanted","color":"f9d0c4"}],"state":"open","locked":false,"assignee":{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false},"assignees":[{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/milestones/1","html_url":"https://github.com/MRtrix3/mrtrix3/milestones/MRtrix3%203.0%20release%20","labels_url":"https://api.github.com/repos/MRtrix3/mrtrix3/milestones/1/labels","id":1036648,"number":1,"title":"MRtrix3 3.0 release ","description":"","creator":{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false},"open_issues":21,"closed_issues":123,"state":"open","created_at":"2015-03-24T16:09:24Z","updated_at":"2016-08-25T05:06:12Z","due_on":"2016-08-31T23:00:00Z","closed_at":null},"comments":7,"created_at":"2016-07-08T22:23:08Z","updated_at":"2016-09-05T01:54:30Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/pulls/712","html_url":"https://github.com/MRtrix3/mrtrix3/pull/712","diff_url":"https://github.com/MRtrix3/mrtrix3/pull/712.diff","patch_url":"https://github.com/MRtrix3/mrtrix3/pull/712.patch"},"body":"Changes required to make things work with Eigen 3.3 beta (testing with Eigen 3.2.92). This basically boils down to sorting out memory alignment issues so that whatever CPU instructions are used to vectorise operations don't crash. Eigen tries to handle these issues internally as much as it can, but there is one specific set of circumstances that causes trouble: [fixed-sized vectorisable objects](https://eigen.tuxfamily.org/dox/group__TopicFixedSizeVectorizable.html), in combination with dynamic memory allocation. \r\n\r\nThe main culprit for us in _MRtrix3_ seems to be `Eigen::Transform`, a.k.a. `transform_type`. Basically, the problem is that `new` allocates memory without regard for the alignment required for the object - whereas static allocation does honour the alignment specified (via constructs like [`alignas`](http://en.cppreference.com/w/cpp/language/alignas)). This isn't a problem for variable-sized matrices, since the memory to hold the contents is dynamically allocated by the Matrix constructor, which takes care of alignment. But that's not the case with fixed-sized objects like `Eigen::Vector4f`, which basically boils down to a `float[4]` static array. \r\n\r\nThe problem is that this propagates through to any class that has such as type as a member. This includes `Header`, and anything that derives from it or contains a `Header` member, but also other classes like the interpolator classes, since they all contain `transform_type` objects. To sort this out, the simplest thing is to define a custom `operator new` that does guarantee alignment. Eigen provides a convenient macro `EIGEN_MAKE_ALIGNED_OPERATOR_NEW` that does precisely this. But this need to be done for every class affected - even if it derives from a class that has already been 'fixed', since the base class's `operator new` will only be invoked for that exact type. \r\n\r\nThere is also an issue with using these types (or classes that contain them) in `std::vector` arrays and other STL containers, but that's a relatively rare occurence.\r\n\r\nSo I've gone through and added this macro to just about every class that was causing problems in testing. Everything checks out as far as I can see, apart from:\r\n\r\n- `tckconvert`: due to minor differences in precision, causing the diff on VTK output to fail - [as discussed recently on the forum](http://community.mrtrix.org/t/errors-from-run-tests-on-mrtrix-3-1-5-on-ubuntu-16-04/340?u=jdtournier);\r\n\r\n- `dwidenoise`: should be fixed by #711, I'll merge that in later;\r\n\r\n- `tckmap`: it's off by more than the precision, but not by a huge margin either - might be down to tiny differences in the rounding of the position of streamline vertices - worth looking into at any rate. \r\n\r\nWhile these changes sort out all issues picked up in testing, there's bound to be cases that I've missed and aren't covered by the testing. It may be that the simplest thing would simply be to mandate that all classes should include the `EIGEN_MAKE_ALIGNED_OPERATOR_NEW` macro just to avoid future problems - I'm pretty sure all it would do is ensure dynamic allocation respects the class's stated alignment requirements, whatever they may be.\r\n\r\nClearly, this is going to need some thorough testing and quite a few more pairs of eyes to make sure everything does work as expected. Although this shouldn't affect operation with Eigen 3.2.8, so it might be safe to merge, and we can pick up other issues downstream as 3.3 beta users report them - not ideal, but there hopefully won't be too many users at this stage...?"},"comment":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/comments/244643372","html_url":"https://github.com/MRtrix3/mrtrix3/pull/712#issuecomment-244643372","issue_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/712","id":244643372,"user":{"login":"thijsdhollander","id":11952694,"avatar_url":"https://avatars.githubusercontent.com/u/11952694?v=3","gravatar_id":"","url":"https://api.github.com/users/thijsdhollander","html_url":"https://github.com/thijsdhollander","followers_url":"https://api.github.com/users/thijsdhollander/followers","following_url":"https://api.github.com/users/thijsdhollander/following{/other_user}","gists_url":"https://api.github.com/users/thijsdhollander/gists{/gist_id}","starred_url":"https://api.github.com/users/thijsdhollander/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thijsdhollander/subscriptions","organizations_url":"https://api.github.com/users/thijsdhollander/orgs","repos_url":"https://api.github.com/users/thijsdhollander/repos","events_url":"https://api.github.com/users/thijsdhollander/events{/privacy}","received_events_url":"https://api.github.com/users/thijsdhollander/received_events","type":"User","site_admin":false},"created_at":"2016-09-05T01:54:30Z","updated_at":"2016-09-05T01:54:30Z","body":"Good, we'll do that.  Apart from advising against using the beta (which I agree with), let's hope these changes already resolve some of the issues with the beta.  There's bound to be users out there who may be using the beta without knowing (specifically those on Ubuntu 6.04)... better to have (some) issues resolved for them than not, I guess."}}
{"actor":{"display_login":"thijsdhollander"}}