Skip to content
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

bug in core/halton.py #449

Closed
BlazejGrabowski opened this issue Dec 21, 2022 · 3 comments · Fixed by #495
Closed

bug in core/halton.py #449

BlazejGrabowski opened this issue Dec 21, 2022 · 3 comments · Fixed by #495

Comments

@BlazejGrabowski
Copy link
Contributor

calling
pts = halton(4, 49, scramble=False)
produces a zero in pts[48,3] which is wrong

the correct value is 0.00291545

the problem occurs in the mod_matrix calculation when calling
np.floor(sum_matrix)
for the problematic entry, sum_matrix is 0.9999999999999999 and floor brings it down to 0, while it should result in 1

adding an epsilon at float precision to sum_matrix does the job, but I have not checked whether it can cause a problem at another place

version: pycalphad 0.10.1

@richardotis
Copy link
Collaborator

Hi Blazej, thanks for your report and analysis. Your solution should work fine without significant consequences. Would you like to submit a pull request with the fix?

@BlazejGrabowski
Copy link
Contributor Author

Hi Richard, if you don't mind, I'd leave the fix to you
Thx for pycalphad, it's a fantastic tool, in particular for teaching

@richardotis
Copy link
Collaborator

Sure, I'll try to get to it soon.

BlazejGrabowski added a commit to BlazejGrabowski/pycalphad that referenced this issue Oct 27, 2023
adding an epsilon at float precision to sum_matrix 
correction according to issue pycalphad#449
@BlazejGrabowski BlazejGrabowski mentioned this issue Oct 27, 2023
4 tasks
@richardotis richardotis linked a pull request Oct 28, 2023 that will close this issue
4 tasks
richardotis pushed a commit that referenced this issue Oct 28, 2023
adding an epsilon at float precision to sum_matrix 
correction according to issue #449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants