-
Notifications
You must be signed in to change notification settings - Fork 270
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
Added C++ backend for Node, TreeNode, ArrayForTrees, BinaryTree and BinarySearchTree and all tree traversals implemented #556
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work. I have left only one question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than this, its ready from my side.
|
||
b.delete(-13) | ||
b.delete(-10) | ||
b.delete(-3) | ||
b.delete(-13) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why these assert
are removed? You said something about it in chat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct return value is True (as delete operation happens). It works correctly on my local (which has Python 3.8), but only for the Python 3.8 CI, return value is None. I have opened up an issue for this: #558
I have skimmed through all the changes. Requested one change and left one question. |
I have updated the benchmarks tests, but now, as we pass the same tree to insert(), search() and delete(), we can run them only once (and not 4 times and then take the min). This is because every time we run the functions |
PyObject* walk = PyTuple_GetItem(tup, 0); | ||
PyObject* parent = PyTuple_GetItem(tup, 1); | ||
Py_INCREF(Py_None); | ||
PyObject* a = Py_None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be this never got updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try setting it to PyObject* a = NULL
; That should raise an error, if it never gets updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value of a is set properly. I checked by printing the value of a (after the statement where it should be updated).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check all the points where this function returns None.
Py_INCREF(Py_None); | ||
PyObject* a = Py_None; | ||
if (walk == Py_None) { | ||
Py_RETURN_NONE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one point where the return None
can happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I did that.
|
||
if (balancing_info != Py_None) { | ||
if (PyLong_AsLong(balancing_info) == 1) { | ||
return a; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is another point. Add print statements and check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried that too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. Great work.
Related to: Google Summer of Code 2024, Kishan Ved
Related issues: #554 and #550
Progress:
Created the C++ backend for:
I am Speed!
C++ backend runs at an extremely fast speed, when compared to the Python backend. Here's an example:
Output
What's working (everything in the C++ backend):
TreeNode C++ backend test:
BinaryTrees C++ backend test:
ArrayForTrees C++ backend test:
BinarySearchTree C++ backend test:
search()
insert()
delete()
,_simple_path()
,lowest common ancestor _lca_1()
,lowest common ancestor _lca_2()
,rank()
,select()
,lower_bound
andupper_bound