-
-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the Bisection Issue in five_preferences.md
#148
Conversation
Hi @mmcky, This PR should fix the issue. It was quite hard to find : ) |
Nice investigative work @HumphreyYang So is the new Before I merge I would just like to understand what the issue was. Thanks @HumphreyYang |
Hi @mmcky, Thanks for asking. I am very happy to go though my long journey last night. Essentially, I looked over all the changes in The root cause of the error lies in the convergence of the line
in function
The differences in the result of
under [ nan nan nan nan nan nan
nan nan nan 4.27158534 3.9723196 3.70486456
3.46477285 3.24835709 3.0525381 2.87472741 2.71273568 2.56470044
2.4290288 2.30435159 2.18948637 2.08340748 1.98522151 1.89414719
1.8094988 1.73067237 1.65713414 1.58841091 1.52408195 nan
1.46562282 1.44679935 1.42862095 1.41105227 1.39406062 1.37761566
1.36168924 1.34625517 1.33128907 1.31676821 1.30267134 1.28897861
1.27567144 1.2627324 1.25014515 1.23789433 1.22596549 1.21434504
1.20302016 1.19197878 1.18120948 1.17070147 1.16044457 1.15042911
1.14064595 1.13108641 1.12174227 1.1126057 1.10366929 1.09492597
1.08636901 1.07799201 1.06978888 1.0617538 1.0538812 1.04616581
nan nan nan nan nan nan
nan nan nan nan nan nan
nan nan nan nan nan nan
nan nan nan nan nan nan
nan nan nan nan nan nan
nan nan nan nan nan] but under [ nan nan nan nan nan nan
nan nan nan 4.27158534 3.9723196 3.70486456
3.46477285 3.24835709 3.0525381 2.87472741 2.71273568 2.56470044
2.4290288 2.30435159 2.18948637 2.08340748 1.98522151 1.89414719
1.8094988 1.73067237 1.65713414 1.58841091 1.52408195 1.48512961
1.46562282 1.44679935 1.42862095 1.41105227 1.39406062 1.37761566
1.36168924 1.34625517 1.33128907 1.31676821 1.30267134 1.28897861
1.27567144 1.2627324 1.25014515 1.23789433 1.22596549 1.21434504
1.20302016 1.19197878 1.18120948 1.17070147 1.16044457 1.15042911
1.14064595 1.13108641 1.12174227 1.1126057 1.10366929 1.09492597
1.08636901 1.07799201 1.06978888 1.0617538 1.0538812 1.04616581
1.03860254 1.03118657 1.02391326 1.01677819 1.00977711 1.00290597
0.99616087 0.98953807 0.98303401 0.97664523 0.97036844 0.96420046
0.95813826 0.95217889 0.94631955 0.94055751 0.93489016 0.929315
0.92382958 0.91843159 0.91311876 0.90788892 0.90273998 0.89766992
0.89267677 0.88775864 0.88291373 0.87814026 0.87343652 0.86880087
0.86423172 0.85972751 0.85528676 0.85090802 0.84658989] Therefore, I suspect that there is a change in the way Indeed, in Below is the convergence at different points under different Under
Under
Therefore, this is a very subtle issue : ) |
thanks @HumphreyYang that's a great history. I guess I was wondering if we should try and fix the function |
Hi @mmcky, I think the function is all good. It is just how scipy handles invalid values in the bracket has changed. It returns NaN directly now. |
I guess where I am going with the function is should we add code to check for |
Hi @mmcky,
We can definitely do that. I think you are suggesting to "restore" what the Many thanks in advance. |
@HumphreyYang let's meet up briefly to discuss. As I understand it -- What I am thinking is if we can add in some code to make the function more robust if a condition is met the SciPy solver is not well behaved or cannot solve. Are there some tests that can be performed (boundary checks)? -- there may not and that is fine (if that is the case). |
@HumphreyYang I will merge this for now -- but let's chat. This is less of an issue then if this was in |
This PR resolves #147.