I want to check what goes into a pull request for sumpy#197
I want to check what goes into a pull request for sumpy#197hirish99 wants to merge 3 commits intoinducer:mainfrom
Conversation
|
Thanks! Could you find a typo somewhere and submit a (separate) PR to fix it? That way, I won't have to approve your PR CI runs one-by-one. |
sumpy/recurrence.py
Outdated
| Copyright (C) 2024 Andreas Kloeckner | ||
| Copyright (C) 2024 Hirish Chandrasekaran |
There was a problem hiding this comment.
Your copyright goes first, you're the main author here.
sumpy/recurrence.py
Outdated
| def make_sympy_vec(name, n): | ||
| return make_obj_array([sp.Symbol(f"{name}{i}") for i in range(n)]) |
There was a problem hiding this comment.
This already exists in sumpy.symbolic. Import from there.
| from pytools.obj_array import make_obj_array | ||
|
|
||
|
|
||
| __doc__ = """ |
There was a problem hiding this comment.
Make sure this is included in the main documentation somewhere, via
.. automodule:: sumpy.recurrence
There was a problem hiding this comment.
I don't understand, do I add this line to the doc string?
sumpy/recurrence.py
Outdated
| ''' | ||
| get_pde_in_recurrence_form | ||
| Input: | ||
| - pde a LinearPDESystemOperator such that assert(len(pde.eqs) == 1) is true. |
There was a problem hiding this comment.
Make sure to use proper references here, e.g.
:class:`sumpy.expansion.diff_op.LinearSystemPDEOperator`
| def make_sympy_vec(name, n): | ||
| return make_obj_array([sp.Symbol(f"{name}{i}") for i in range(n)]) | ||
|
|
||
| class Recurrence: |
There was a problem hiding this comment.
Is there a good reason this should be a class? (vs. just a function)
There was a problem hiding this comment.
I think so. Right now the code is structured so that you have a sequence of functions where the output of one function goes into the input of another function. There are intermediate representations that I am interested in like the ODE form of the PDE. If I create a recurrence class then I can store all the intermediate representations as attributes and get them immediately if I am interested in them.
2 participants
Added a simple class called recurrence that takes in a sumpy PDE and outputs a recurrence relation. However the class is not completely unit-tested or even complete in functionality.