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

Create GeneralOptimizer class and fix index error in Pyramid class #151

Merged
merged 2 commits into from
Jul 5, 2018

Conversation

whzup
Copy link
Collaborator

@whzup whzup commented Jul 2, 2018

Created a GeneralOptimizer class for the implementation of new topologies. Updated the documentation and created a new test file for the new class. With the new test file, it became possible to test the Pyramid class (topology) more easily.
Fixed an index error that occurred in the Pyramid topology because of a type mismatch.

Resolves: #148

whzup added 2 commits July 2, 2018 13:37
Created a GeneralOptimizer class with a topology parameter so one can choose which topology to use. Some special additions implemented:

 - Added an error message in case the topology attribute has the wrong type.
 - Added a special case for the ring topology so it's the same as local optimization.

Also, created the corresponding test file and updated the documentation as well as the RST files for the GeneralOptimizer as well as the Pyramid topology.

Resolves: #148
Fixed an index error in the Pyramid class that occured because the type of the idx array did not match 'int'
@ljvmiranda921 ljvmiranda921 self-requested a review July 3, 2018 08:25
@ljvmiranda921 ljvmiranda921 added enhancement Feature requests documentation Documentation improvements or fixes v0.3.0 labels Jul 3, 2018
Copy link
Owner

@ljvmiranda921 ljvmiranda921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for this @whzup ! Hopefully I can merge this next week Tuesday! Just need to try this implementation again.

@whzup
Copy link
Collaborator Author

whzup commented Jul 4, 2018

Nice, no problem @ljvmiranda921! By the way, I already have a Random topology almost ready for a PR. It's built on the branch used for this PR so I'll wait until you merge this one before making the next.

@whzup
Copy link
Collaborator Author

whzup commented Jul 4, 2018

Should I include a citation to this paper in the Pyramid class doc string before you merge it? It might give us some scientific "back-up" for the implementation of the Delaunay triangulation.

@ljvmiranda921
Copy link
Owner

Hi @whzup , I'll merge this one first. I will open up a new issue regarding documentation fixes, including the one you mentioned in #152 so that we can start preparing for v.0.3.0

@ljvmiranda921 ljvmiranda921 merged commit 3d5e346 into ljvmiranda921:development Jul 5, 2018
@ljvmiranda921
Copy link
Owner

Merged! Thanks for this @whzup ! You're awesome!

@whzup whzup deleted the gen_opt branch July 5, 2018 16:50
@ljvmiranda921 ljvmiranda921 mentioned this pull request Jul 31, 2018
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements or fixes enhancement Feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants