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

Implement a more robust rounding algorithm (no magic number) #157

Closed
wants to merge 1 commit into from

Conversation

basnijholt
Copy link
Contributor

@basnijholt basnijholt commented Jun 27, 2022

I have checked this on large designs and found this to work more robustly.

The previous algorithm failed when creating devices on different machines, where the arrays were basically identical, but the rounding ensured that the values changed.

Let me know if you are interested in merging this PR, then I will update the hashes in the tests.

I have checked this on large designs and found this to work more robustly.
@amccaugh
Copy link
Owner

amccaugh commented Jul 24, 2022

I like the idea of a more robust hashing algorithm, but can you explain what's different here? As far as I can tell, the only difference is that it's using np.array.round(points) instead of implicit rounding by casting the array type to Int64. I don't think there's a meaningful difference between the two operations, are you sure this is really more robust? Are there any tests you can demonstrate to show that it is?

This code also has the side-effect that it's only possible to use precisions which are 10^x (0.1, 1e-4, etc). I don't think that's a huge deal really but it is a reduction in functionality

@basnijholt
Copy link
Contributor Author

basnijholt commented Aug 13, 2022

I have tried this on our internal code base and noticed it produced more reliable hashing when making changes to the environments.

I will try to come up with a simple reproducible example and test (soonish).

@joamatab
Copy link
Contributor

How about rounding to 1nm precision?

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 this pull request may close these issues.

3 participants