-
Notifications
You must be signed in to change notification settings - Fork 15
Add QBMAX expansions #233
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
base: main
Are you sure you want to change the base?
Add QBMAX expansions #233
Conversation
This and #232 look just about identical. I think only one of them is needed? |
Just include |
sumpy/expansion/local.py
Outdated
|
||
|
||
# {{{ Asymline taylor | ||
class AsymLineTaylorLocalExpansion(LocalExpansionBase): |
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.
Add documentation describing what this does.
sumpy/expansion/local.py
Outdated
|
||
|
||
# {{{ Asymline taylor | ||
class AsymLineTaylorLocalExpansion(LocalExpansionBase): |
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.
Please add tests.
sumpy/qbx.py
Outdated
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.
Parts of this look like it interferes with/duplicates/undoes #229. Could you clarify the relationship? We should have one PR per change; it's not great to gobble multiple changes into one.
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.
You’re right — the changes in qbx.py overlap with those in #229. I’ve removed them from this PR to avoid duplication.
4a52f96
to
2764266
Compare
I also set CACHING_ENABLED = False in sumpy/tools.py. For different$\tau$ values, do we need to create separate cache files for the kernels?