Skip to content

Make TargetPointMultiplier a dataclass#269

Merged
inducer merged 1 commit intoinducer:mainfrom
alexfikl:dataclass-target-multiplier
Mar 16, 2026
Merged

Make TargetPointMultiplier a dataclass#269
inducer merged 1 commit intoinducer:mainfrom
alexfikl:dataclass-target-multiplier

Conversation

@alexfikl
Copy link
Collaborator

It has an additional field, so it needs the decorator.

@alexfikl alexfikl force-pushed the dataclass-target-multiplier branch from c711e64 to 742c7ac Compare March 16, 2026 11:31
@alexfikl
Copy link
Collaborator Author

alexfikl commented Mar 16, 2026

FYI, between this and the __reduce__ changes, the Stokes PR is passing locally 🎉

@inducer inducer force-pushed the dataclass-target-multiplier branch from 742c7ac to 714ee07 Compare March 16, 2026 13:48
@inducer
Copy link
Owner

inducer commented Mar 16, 2026

FYI, between this and the __reduce__ changes, the Stokes PR is passing locally 🎉

Oh cool! How come? It looked like the Stokes PR had numerical issues, but these weren't numerical changes? 🤔

@inducer inducer enabled auto-merge (rebase) March 16, 2026 13:49
@alexfikl
Copy link
Collaborator Author

Oh cool! How come? It looked like the Stokes PR had numerical issues, but these weren't numerical changes? 🤔

As far as I can tell, because this didn't include the axis, it would hash the same for every axis. That would then confuse the OperatorCompiler that would consider IntGs with different kernels to actually be the same.. and then numerical chaos 😁

@inducer
Copy link
Owner

inducer commented Mar 16, 2026

Oh dear. Good catch! That would do it 🙂. All the more important to dataclass all the things.

@alexfikl
Copy link
Collaborator Author

Oh dear. Good catch! That would do it 🙂. All the more important to dataclass all the things.

Still not quite sure why this only caused issues for the single-layer Stokes / elasticity, but oh well..

@inducer
Copy link
Owner

inducer commented Mar 16, 2026

Still not quite sure why this only caused issues for the single-layer Stokes / elasticity, but oh well..

It didn't break other parts of pytential because they aren't using this.

@alexfikl
Copy link
Collaborator Author

It didn't break other parts of pytential because they aren't using this.

Yeah, but the Stokes + Laplace + double-layer (in StressletWrapperTornberg) also uses this and it seemed to be converging just fine. I imagine some things were cancelling just right for it..

@inducer inducer merged commit a969c89 into inducer:main Mar 16, 2026
9 checks passed
@alexfikl alexfikl deleted the dataclass-target-multiplier branch March 16, 2026 14:26
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