{"action":"created","issue":{"url":"https://api.github.com/repos/optimatika/ojAlgo/issues/7","labels_url":"https://api.github.com/repos/optimatika/ojAlgo/issues/7/labels{/name}","comments_url":"https://api.github.com/repos/optimatika/ojAlgo/issues/7/comments","events_url":"https://api.github.com/repos/optimatika/ojAlgo/issues/7/events","html_url":"https://github.com/optimatika/ojAlgo/issues/7","id":104113070,"number":7,"title":"Stack Overflow at ExpressionBasedModel#presolve(ExpressionsBasedModel)","user":{"login":"younes-abouelnagah","id":9809574,"avatar_url":"https://avatars.githubusercontent.com/u/9809574?v=3","gravatar_id":"","url":"https://api.github.com/users/younes-abouelnagah","html_url":"https://github.com/younes-abouelnagah","followers_url":"https://api.github.com/users/younes-abouelnagah/followers","following_url":"https://api.github.com/users/younes-abouelnagah/following{/other_user}","gists_url":"https://api.github.com/users/younes-abouelnagah/gists{/gist_id}","starred_url":"https://api.github.com/users/younes-abouelnagah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/younes-abouelnagah/subscriptions","organizations_url":"https://api.github.com/users/younes-abouelnagah/orgs","repos_url":"https://api.github.com/users/younes-abouelnagah/repos","events_url":"https://api.github.com/users/younes-abouelnagah/events{/privacy}","received_events_url":"https://api.github.com/users/younes-abouelnagah/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"apete","id":13863596,"avatar_url":"https://avatars.githubusercontent.com/u/13863596?v=3","gravatar_id":"","url":"https://api.github.com/users/apete","html_url":"https://github.com/apete","followers_url":"https://api.github.com/users/apete/followers","following_url":"https://api.github.com/users/apete/following{/other_user}","gists_url":"https://api.github.com/users/apete/gists{/gist_id}","starred_url":"https://api.github.com/users/apete/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apete/subscriptions","organizations_url":"https://api.github.com/users/apete/orgs","repos_url":"https://api.github.com/users/apete/repos","events_url":"https://api.github.com/users/apete/events{/privacy}","received_events_url":"https://api.github.com/users/apete/received_events","type":"User","site_admin":false},"milestone":null,"comments":6,"created_at":"2015-08-31T18:58:52Z","updated_at":"2015-09-03T07:33:26Z","closed_at":null,"body":"Hi,\r\n\r\nFirst of all, thanks for open sourcing this promising library. I am evaluating it against JOptimise and I found that the current implementation of the presolve function in ExpressionsBasedModel causes a stack overflow because of the recursive call on line 125 (version 38). \r\n\r\nI suggest doing 3 things:\r\n\r\n1) use this code instead:\r\n```\r\n    /**\r\n     * Avoids stack overflow exceptions caused by the original  {@link ExpressionsBasedModel#presolve()}.\r\n     *\r\n     * @param model\r\n     */\r\n    public static final void presolveIterative(final ExpressionsBasedModel model) {\r\n\r\n        int iters = 0;\r\n        boolean stillSimplifying = true;\r\n        while (stillSimplifying) {\r\n            ++iters;\r\n            if (iters % 100 == 0) {\r\n                log.info(\"Done {} iterations of presolving\");\r\n            }\r\n\r\n            stillSimplifying = false;\r\n\r\n            for (final Expression tmpExpression : model.getExpressions()) {\r\n                if (tmpExpression.simplify()) {\r\n                    log.debug(\"Following expression is now redundant: {}\", tmpExpression);\r\n                    stillSimplifying = true;\r\n                    break; // Restart the process after removing something.\r\n                }\r\n            }\r\n        }\r\n    }\r\n```\r\n\r\n2) Make the presolve functions normal methods.. you have one presolve function as final and the other as static, which prevents subclasses from overriding either. This makes it unnecessarily harder to extend your library, and yet not impossible.\r\n\r\n3) Create a tag on github for each release, so that anybody can fork the version they are using and submit a bug fix pull request. Right now I would need to pull master and fix the bug in a version that is unstable and not ready for release. \r\n\r\nThanks again."},"comment":{"url":"https://api.github.com/repos/optimatika/ojAlgo/issues/comments/137360877","html_url":"https://github.com/optimatika/ojAlgo/issues/7#issuecomment-137360877","issue_url":"https://api.github.com/repos/optimatika/ojAlgo/issues/7","id":137360877,"user":{"login":"apete","id":13863596,"avatar_url":"https://avatars.githubusercontent.com/u/13863596?v=3","gravatar_id":"","url":"https://api.github.com/users/apete","html_url":"https://github.com/apete","followers_url":"https://api.github.com/users/apete/followers","following_url":"https://api.github.com/users/apete/following{/other_user}","gists_url":"https://api.github.com/users/apete/gists{/gist_id}","starred_url":"https://api.github.com/users/apete/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apete/subscriptions","organizations_url":"https://api.github.com/users/apete/orgs","repos_url":"https://api.github.com/users/apete/repos","events_url":"https://api.github.com/users/apete/events{/privacy}","received_events_url":"https://api.github.com/users/apete/received_events","type":"User","site_admin":false},"created_at":"2015-09-03T07:33:26Z","updated_at":"2015-09-03T07:33:26Z","body":"If you're using v38.x you really should be using v38.2. I have created a tag for v38.2 (the presolve functionality has been improved) and there shouldn't be any problem evaluating the optimisation parts of ojAlgo using the current/master/head.\r\n\r\nIf you want to submit a pull request for an iterative presolver, then go ahead. I have been thinking about refactoring the presolve implementation so that users can turn it off and/or add different presolve modules. When I do this I'll make sure it is iterative rather than recursive..."}}