Skip to content

Question about exercise 4.9 #14

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

Closed
edsml-sz2421 opened this issue Aug 15, 2021 · 8 comments
Closed

Question about exercise 4.9 #14

edsml-sz2421 opened this issue Aug 15, 2021 · 8 comments
Labels
bug Something isn't working JupyterHub JupyterHub-related issue PyBryt PyBryt-related issue waiting-PyBryt-release Waiting for PyBryt release.

Comments

@edsml-sz2421
Copy link

Hi, could you please check the result of the 4.9 cuz I really don't know what's wrong with it. I passed the test case but not for the pybryt check. I don't think I get the wrong roots for the question. Could you please help me check it?
image

@marijanbeg
Copy link
Contributor

Hi @edsml-4c29515f, thank you for your question and we are sorry you encountered this problem.

Could you please send us the code you are testing so that we can investigate further?

@marijanbeg marijanbeg added awaiting-response Additional info needed PyBryt PyBryt-related issue labels Aug 15, 2021
@edsml-sz2421
Copy link
Author

Hope you help me check it. Thanks a lot!

import numpy as np
class Quadratic:
      def __init__(self, a, b, c):
          self.a = a
          self.b = b
          self.c = c
  
      def value(self, x):
          return self.a * x**2 + self.b * x + self.c
  
      def table(self, L, R, n):
          p = np.linspace(L, R, n)
          for i in range(0, n):
              print('x: %g\tf: %g' % (p[i], self.value(p[i])))
  
      def roots(self):
          if (self.a == 0):
              return -c/b
          if (self.b**2 - 4 * self.a * self.c < 0):
              return None
          x1 = (-self.b - math.sqrt(self.b**2 - 4 * self.a * self.c)) / (2 * self.a)
          x2 = (-self.b + math.sqrt(self.b**2 - 4 * self.a * self.c)) / (2 * self.a)
          return [x1, x2]

@marijanbeg
Copy link
Contributor

Hi @edsml-4c29515f, thank you very much for the clarification. Your solution is correct, and it passed the okpy test.

The reason you receive errors from PyBryt is due to the bug we discovered recently. More precisely, tolerances for iterable data types (lists, tuples, NumPy arrays, etc.) were not implemented in PyBryt correctly. A similar issue has been reported in #12. Since then, this bug has been fixed (microsoft/pybryt#103) and the new PyBryt version released.

Depending on how you are running notebooks, there are several solutions:

  1. JupyterHub: You do not need to do anything, and PyBryt will be updated soon. We appreciate your patience.
  2. Locally (on your computer): you can update PyBryt by running
pip install --upgrade pybryt

Before running this command, please make sure you are in the correct conda environment.
3. Binder: You do not need to do anything because the newest version of PyBryt is installed when you start the new session.

Finally, although we did not cover this in the lecture notebooks, when a function returns more than one result, it is common to return a tuple (x1, x2) and not a list [x1, x2]. So, for instance, if you did not explicitly create a tuple before returning, it would be created automatically since you used a comma in the return statement:

def roots(self):
         ...
          return x1, x2  # returns a tuple (x1, x2) even though we did not use parentheses

We will clarify this in the notebook.

I will keep this issue open until PyBryt on JupyterHub is updated, and we receive your confirmation that the issue is resolved. Again, thank you very much for your valuable feedback, and please do not hesitate to raise further issues.

@marijanbeg marijanbeg added JupyterHub JupyterHub-related issue waiting-PyBryt-release Waiting for PyBryt release. bug Something isn't working and removed awaiting-response Additional info needed labels Aug 15, 2021
@marijanbeg
Copy link
Contributor

For reference, return type clarification has been addressed in #15. Merged to master.

@marijanbeg
Copy link
Contributor

Hi @edsml-4c29515f, I can confirm that the PyBryt version has been upgraded to 0.1.9 on JupyterHub. Therefore, I am closing this issue now.

Thank you very much for your valuable feedback, and please do not hesitate to raise further issues.

@edsml-ds423
Copy link

Hi @marijanbeg - I hope you don't mind but I wanted to re-open this issue as I think I am having the same problem....

I have the following code:
Screenshot 2023-10-04 at 10 14 51

And I am erroring on the root calculations. The assertion cell below all passes and I have manually calculated the roots and I get the correct values (I think!)

@marijanbeg
Copy link
Contributor

Hi @edsml-8eb3e724, thank you for your question. If assert statements are passing, I would say that your solution is correct. Please note:

Although PyBryt provides much more detailed feedback, it is possible it complains about your code even though assert statement confirms your code is correct. This is because PyBryt testing is based on comparing your solution to reference solutions. Since there is virtually an infinite number of alternative solutions, it is possible that your solution is not in our references. On the other hand, assert statements check the final result of your code and if they do not raise an error, your code is correct and you do not have to address PyBryt's error messages (if any).

@edsml-ds423
Copy link

@marijanbeg - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working JupyterHub JupyterHub-related issue PyBryt PyBryt-related issue waiting-PyBryt-release Waiting for PyBryt release.
Projects
None yet
Development

No branches or pull requests

3 participants