Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Perf tweaks #1

Closed
wants to merge 41 commits into from
Closed

Perf tweaks #1

wants to merge 41 commits into from

Conversation

mortonjt
Copy link

@mortonjt mortonjt commented Aug 4, 2016

Adding in two more test cases to help debugging.

Basically, your code is failing to create an orthogonal basis. I have a really small example put together

          /-O0
         |
         |          /-O1
-y0------|         |
         |         |                                        /-O8
         |         |                              /y7------|
         |         |                             |          \-O9
          \y1------|                    /y5------|
                   |                   |         |          /-O6
                   |                   |          \y8------|
                   |          /y3------|                    \-O7
                   |         |         |
                   |         |         |          /-O4
                    \y2------|          \y6------|
                             |                    \-O5
                             |
                             |          /-O2
                              \y4------|
                                        \-O3

Will lead to the following basis

          O0        O1        O2        O3        O4        O5        O8        O9        O6        O7  
y0  0.037280  0.106969  0.106969  0.106969  0.106969  0.106969  0.106969  0.106969  0.106969  0.106969   
y1  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000   
y2  0.096245  0.037491  0.108283  0.108283  0.108283  0.108283  0.108283  0.108283  0.108283  0.108283   
y3  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000  0.100000   
y4  0.094502  0.094502  0.077053  0.077053  0.077053  0.077053  0.077053  0.077053  0.174338  0.174338   
y5  0.094700  0.094700  0.070955  0.070955  0.070955  0.070955  0.168691  0.168691  0.094700  0.094700   
y6  0.095046  0.095046  0.095046  0.095046  0.095046  0.095046  0.095046  0.095046  0.046864  0.192765   
y7  0.095143  0.095143  0.057707  0.057707  0.156864  0.156864  0.095143  0.095143  0.095143  0.095143   
y8  0.095046  0.095046  0.095046  0.095046  0.095046  0.095046  0.046864  0.192765  0.095046  0.095046   

So y1 and y4 are exactly identical.

Will do some more digging into the code. Thanks for putting together!

wasade and others added 30 commits July 10, 2016 20:48
Last touches before release
Correcting documentation in setup.py
Adding tests for duplicate ids, updating documentation.
Adding headers for copyright
Adding some mutability tests
Adding warning about replacing internal node names
@wasade
Copy link
Owner

wasade commented Aug 4, 2016

Against my repo?

On Aug 4, 2016 21:40, "Jamie Morton" notifications@github.com wrote:

Adding in two more test cases to help debugging.

Basically, your code is failing to create an orthogonal basis. I have a
really small example put together

      /-O0
     |
     |          /-O1

-y0------| |
| | /-O8
| | /y7------|
| | | -O9
\y1------| /y5------|
| | | /-O6
| | \y8------|
| /y3------| -O7
| | |
| | | /-O4
\y2------| \y6------|
| -O5
|
| /-O2
\y4------|
-O3

Will lead to the following

      O0        O1        O2        O3        O4        O5        O8        O9        O6        O7

y0 0.037280 0.106969 0.106969 0.106969 0.106969 0.106969 0.106969 0.106969 0.106969 0.106969
y1 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000
y2 0.096245 0.037491 0.108283 0.108283 0.108283 0.108283 0.108283 0.108283 0.108283 0.108283
y3 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000 0.100000
y4 0.094502 0.094502 0.077053 0.077053 0.077053 0.077053 0.077053 0.077053 0.174338 0.174338
y5 0.094700 0.094700 0.070955 0.070955 0.070955 0.070955 0.168691 0.168691 0.094700 0.094700
y6 0.095046 0.095046 0.095046 0.095046 0.095046 0.095046 0.095046 0.095046 0.046864 0.192765
y7 0.095143 0.095143 0.057707 0.057707 0.156864 0.156864 0.095143 0.095143 0.095143 0.095143
y8 0.095046 0.095046 0.095046 0.095046 0.095046 0.095046 0.046864 0.192765 0.095046 0.095046


You can view, comment on, or merge this pull request online at:

#1
Commit Summary

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAc8spwrBjh1D09ZZ0in9J7SjRjV9E5kks5qcjIJgaJpZM4Jc_1R
.

@mortonjt
Copy link
Author

mortonjt commented Aug 4, 2016

Meant to do it against your fork ...

@mortonjt mortonjt closed this Aug 4, 2016
@mortonjt mortonjt mentioned this pull request Aug 4, 2016
@mortonjt mortonjt deleted the perf_tweaks branch April 6, 2017 11:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants