{"action":"created","issue":{"url":"https://api.github.com/repos/idaholab/moose/issues/8659","repository_url":"https://api.github.com/repos/idaholab/moose","labels_url":"https://api.github.com/repos/idaholab/moose/issues/8659/labels{/name}","comments_url":"https://api.github.com/repos/idaholab/moose/issues/8659/comments","events_url":"https://api.github.com/repos/idaholab/moose/issues/8659/events","html_url":"https://github.com/idaholab/moose/issues/8659","id":211451969,"number":8659,"title":"DependencyResolver<shared_ptr> does not sort reliably.","user":{"login":"roystgnr","id":2367346,"avatar_url":"https://avatars2.githubusercontent.com/u/2367346?v=3","gravatar_id":"","url":"https://api.github.com/users/roystgnr","html_url":"https://github.com/roystgnr","followers_url":"https://api.github.com/users/roystgnr/followers","following_url":"https://api.github.com/users/roystgnr/following{/other_user}","gists_url":"https://api.github.com/users/roystgnr/gists{/gist_id}","starred_url":"https://api.github.com/users/roystgnr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/roystgnr/subscriptions","organizations_url":"https://api.github.com/users/roystgnr/orgs","repos_url":"https://api.github.com/users/roystgnr/repos","events_url":"https://api.github.com/users/roystgnr/events{/privacy}","received_events_url":"https://api.github.com/users/roystgnr/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2017-03-02T16:55:02Z","updated_at":"2017-03-02T17:44:42Z","closed_at":null,"body":"### Description of the enhancement or error report\r\n\r\nWhen calling DependencyResolver<T>::getSortedValues() we expect to get a vector which is consistently sorted across processors.  If we do not, then e.g. in MooseApp::executeMeshModifiers() we will end up executing MeshModifiers in different orders on different processors, which is a disaster for any mesh modifier code that does parallel communication.\r\n\r\ngetSortedValues() does two levels of sorting: an outer level based on dependencies (which works reliably) and an inner level based on the ordering within _ordered_items[i].  For std::shared_ptr items, this ordering is based on memory allocation, which is surprisingly consistent across processors but which occasionally breaks horribly.\r\n\r\n### Rationale for the enhancement or information for reproducing the error\r\n\r\nRunning BlockDeleterTest11 (which has lots of MeshModifiers) with --distributed-mesh (which makes half of those MeshModifiers require parallel communication) on many processors (to maximize the chance that one of them sorts out of the intended order and the likelihood that a mis-synced deleter will be querying about an element which was already deleted by a different deleter) in debug or devel mode (to cause those queries to fail assertions) usually (but not deterministically!) replicates the problem.\r\n\r\n### Identified impact\r\n\r\nThe only direct use of DependencyResolver is for mesh modifiers, but DependencyResolverInterface uses std::stable_sort with a DependencyResolver as comparator and so should have the same bug.\r\n\r\nThat makes this a *very* intrusive issue, which is why I'm making a new issue rather than rolling out a fix under the #1500 banner.\r\n\r\nI'd *like* to do the internal sort by name.  I'll try that shortly and see whether the compiler screams.\r\n\r\nUsing T::name() essentially requires assuming that every single DependencyResolver use is with a MooseObject subclass, and I can't tell that for certain; DependencyResolverInterface gets brought in *everywhere* via multiple inheritance, and trying to examine the results of \"grep sort\" in its subclasses is slow work.\r\n\r\nEven if this compiles, I can imagine it changing regression test results, as it may change the result of sorts:  If you added object \"ObjectTwo\" and \"ObjectThree\" in that order, then previously the sort would (probably! not definitely!) put ObjectTwo first because it ends up at a lower memory address, and in the future the sort would always return ObjectThree first because of alphabetical order."},"comment":{"url":"https://api.github.com/repos/idaholab/moose/issues/comments/283725549","html_url":"https://github.com/idaholab/moose/issues/8659#issuecomment-283725549","issue_url":"https://api.github.com/repos/idaholab/moose/issues/8659","id":283725549,"user":{"login":"permcody","id":3036960,"avatar_url":"https://avatars1.githubusercontent.com/u/3036960?v=3","gravatar_id":"","url":"https://api.github.com/users/permcody","html_url":"https://github.com/permcody","followers_url":"https://api.github.com/users/permcody/followers","following_url":"https://api.github.com/users/permcody/following{/other_user}","gists_url":"https://api.github.com/users/permcody/gists{/gist_id}","starred_url":"https://api.github.com/users/permcody/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/permcody/subscriptions","organizations_url":"https://api.github.com/users/permcody/orgs","repos_url":"https://api.github.com/users/permcody/repos","events_url":"https://api.github.com/users/permcody/events{/privacy}","received_events_url":"https://api.github.com/users/permcody/received_events","type":"User","site_admin":false},"created_at":"2017-03-02T17:44:42Z","updated_at":"2017-03-02T17:44:42Z","body":"Oh my, I'm afraid this is my doing but sorting on pointers wasn't my intention.\r\n\r\nI don't recall why I'm sorting the inner vector at all. It's probably just my naive choice of using `std::set` as the inner container. It might be easier to rework the DependencyResolver so that the inner holder is a `std::list` or some other unordered container which we don't have to worry about getting shuffled. This change could have bad asymptotic behavior for the algortihm itself but should be fine in practice. We could get fancy and use an indirect sort (also a utility in MOOSE) if we wanted to bring down the runtime bounds. This shouldn't require any changes to the external interface!\r\n\r\n"}}
{"actor":{"display_login":"permcody"}}