Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Kalevi Suominen
    @jksuom
    Ok. Fine.
    Divyanshu Thakur
    @divyanshu132
    @jksuom I'm thinking to provide some kind of explanation, for the order in which presentation is computed with an example, below is an image have a look if it looks fine.
    SymPy.png
    Divyanshu Thakur
    @divyanshu132
    Here is the PR sympy/sympy#17399 have a look.
    Kalevi Suominen
    @jksuom
    The explanation is good but it seems that pc_series was reversed for computational purposes. Perhaps it would be more logical to use the original order in the documentation: For x3**2, pc_series[4], which is trivial, pc_series[3] for x2**2 and x2**-2*x3*x2, etc.
    Divyanshu Thakur
    @divyanshu132
    Yaa, right.
    Kalevi Suominen
    @jksuom
    The documentation PR looks good. Tests have passed. It can probably be merged soon. As to the other one, I think that it would suffice to add a test for the relative orders being primes.
    Divyanshu Thakur
    @divyanshu132

    I think that it would suffice to add a test for the relative orders being primes.

    That kind of tests are already there, should we add more tests.

    I don't understand how Codecov works.
    Kalevi Suominen
    @jksuom
    It seems that the handbook assumes that the relative orders are prime in the induced pcgs algorithm. I tried to look more carefully where it would be needed but could not find such a place. So it looks like the test is not necessary for the algorithm.
    I don't understand either how Codecov works but perhaps we should not worry too much.
    Divyanshu Thakur
    @divyanshu132

    So it looks like the test is not necessary for the algorithm.

    So, Is it good to go?

    Kalevi Suominen
    @jksuom
    I think it can be merged now.
    Divyanshu Thakur
    @divyanshu132
    Thanks!
    Kalevi Suominen
    @jksuom
    The Sphinx documentation now passes the tests. Have you built the html? Does it look ok?
    Divyanshu Thakur
    @divyanshu132
    Yes, it looks good to me.
    Should I share some screenshots?
    Kalevi Suominen
    @jksuom
    That would be helpful for me.
    Divyanshu Thakur
    @divyanshu132
    Ok!
    Divyanshu Thakur
    @divyanshu132
    sympy1.png
    sympy2.png
    sympy3.png
    sympy4.png
    sympy5.png
    Kalevi Suominen
    @jksuom
    Thanks. That looks ok. I left a comment on relative_order that would probably look better as upright text.
    Divyanshu Thakur
    @divyanshu132
    Okay.
    Kalevi Suominen
    @jksuom
    There is one thing that I forgot to mention. Namely, single backticks generate LaTeX (and mathit) in SymPy's Sphinx configuration. That makes code like pcgs and pc_series[4] look different from what we would want. We should use double backticks instead.
    Divyanshu Thakur
    @divyanshu132
    Ok, I'll push the changes by tonight.
    Kalevi Suominen
    @jksuom
    It looks like the rst file is ready to be included though I could not make the html. My sphinx version (ubuntu 16.04) may be too old.
    Divyanshu Thakur
    @divyanshu132
    Do you need few screenshots as per above changes.
    Kalevi Suominen
    @jksuom
    Can you send the page on the computation of polycyclic presentation?
    Divyanshu Thakur
    @divyanshu132
    sympy6.png
    Have a look!
    Kalevi Suominen
    @jksuom
    Thanks. That looks ok to me.
    I think it can be merged.
    Divyanshu Thakur
    @divyanshu132
    Thanks!
    Kalevi Suominen
    @jksuom
    There is one more thing that you could work with. It would be useful to have more information in the docstrings of classes and their methods. In particular, "Parameters" could be added. We generally follow the numpy conventions as explained here.
    There are some differences. Double backticks instead of single ones for variables, for example, and ===== for underlining (with exactly the same length).
    Divyanshu Thakur
    @divyanshu132
    Ok, I'll try!
    Divyanshu Thakur
    @divyanshu132
    @jksuom I've opened a PR on extending the docstrings of polycyclic groups. Have a look sympy/sympy#17476
    Kalevi Suominen
    @jksuom
    Ok, thanks.
    Divyanshu Thakur
    @divyanshu132
    \{ is generating error, can't we use escape sequence in docstrings?
    Kalevi Suominen
    @jksuom
    It should be possible if the docstring is raw: r""" ... """.
    Divyanshu Thakur
    @divyanshu132
    oohh!
    Divyanshu Thakur
    @divyanshu132
    @jksuom I've added the final report, please have a look and let me know if anything needs improvement https://github.com/sympy/sympy/wiki/GSoC-2019-Report
    Kalevi Suominen
    @jksuom
    Ok. I'll do that tomorrow.
    Divyanshu Thakur
    @divyanshu132
    Ok!
    Kalevi Suominen
    @jksuom
    That looks fine to me. (Though I'm not sure of the word "whosoever".) I think that google would expect a link to the actual report https://github.com/sympy/sympy/wiki/GSoC-2019-Report-Divyanshu-Thakur:-Group-Theory. I'll have to check the instructions.
    Divyanshu Thakur
    @divyanshu132
    I'll replace the word "whosoever".

    I think that google would expect a link to the actual report

    Okay!