Skip to content

Conversation

@alexfikl
Copy link
Collaborator

No description provided.

Comment on lines 152 to 155
err_const_helmholtz: float = 100.0
"""Constant used in the Laplace truncation error."""
scaling_const_helmholtz: float = 4.0
"""Constant :math:`C_{\text{helmscale}}` used in the Helmholtz truncation error."""
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understood the difference between err_const_helmholtz and scaling_const_helmholtz. Do these docs make sense?

Copy link
Collaborator Author

@alexfikl alexfikl Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated these a bit to correctly mention the variables in the main class docs, so it should be fine.. hopefully..

https://github.com/inducer/sumpy/compare/2e2360206cc7c51b9fa1f2631412382195587a1c..371cbcacd928780acabc1f8357bf486668bf03d2

Comment on lines 176 to 177
# FIXME: these seem to be sym.Expr in every usage here. Any other cases?
TranslationClassesDepData: TypeAlias = tuple[Any, ...]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe to try out tuple[sym.Expr, ...]?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it typechecks, feel free to make it sharper.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexfikl alexfikl force-pushed the type-expansion branch 2 times, most recently from 9875fce to 9c053e9 Compare November 21, 2025 14:23
@alexfikl alexfikl marked this pull request as ready for review November 21, 2025 19:45
@alexfikl
Copy link
Collaborator Author

alexfikl commented Nov 21, 2025

@inducer This should pass the tests now™, so it's good to take a look at whenever you have the time.

EDIT: I'm assuming that failure is because I sympified some zeros in the FFT code. I'll take a look tomorrow :(

@inducer inducer enabled auto-merge (rebase) November 24, 2025 17:42
@inducer
Copy link
Owner

inducer commented Nov 24, 2025

Thx!

@inducer inducer merged commit e4fa17a into inducer:main Nov 24, 2025
9 checks passed
@alexfikl alexfikl deleted the type-expansion branch December 3, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants