Skip to content

Add PrecomposedSlicedSeparableSum implementation and corresponding tests#157

Open
hakkelt wants to merge 3 commits intoJuliaFirstOrder:masterfrom
hakkelt:precomposed-sliced-separable-sum
Open

Add PrecomposedSlicedSeparableSum implementation and corresponding tests#157
hakkelt wants to merge 3 commits intoJuliaFirstOrder:masterfrom
hakkelt:precomposed-sliced-separable-sum

Conversation

@hakkelt
Copy link
Copy Markdown
Contributor

@hakkelt hakkelt commented Apr 13, 2026

This new function implements the intersection/composition of Precompose and SlicedSeparableSum. The long name suggests a code smell, but I could not find a cleaner way to implement this composition, so I'm open to better solutions. The goal is to implement prox for tricky expressions like the following:

x = (randn(10), randn(10))
norm(x[1], 1) + norm(A2[1:5, 1:5] * x[2][1:5], 2) + norm(A2[6:10, 6:10] * x[2][6:10], 2)^2

@hakkelt hakkelt marked this pull request as ready for review April 13, 2026 17:56
@hakkelt hakkelt marked this pull request as draft April 13, 2026 18:12
@hakkelt hakkelt force-pushed the precomposed-sliced-separable-sum branch from d947f82 to da2eaa3 Compare April 13, 2026 18:29
@hakkelt hakkelt marked this pull request as ready for review April 13, 2026 18:53
@hakkelt
Copy link
Copy Markdown
Contributor Author

hakkelt commented Apr 13, 2026

@lostella: Could you review this PR?

The benchmarking is still failing because of the lack of permission. 😞 My guess is that adding write permisison to workflows (Settings -> Actions -> General -> Workflow permissions -> Read repository contents and packages permissions) might solve the problem. I'm not really expert on security, so I'm not sure how dangerous it is exactly. What I read about that is that it can lead to exposure of secrects (e.g. if someone creates a PR that modifies the benchmarking script such that it sends the GITHUB_TOKEN to an external server). But as your approval is required for PRs of new contributors by the current settings, the threat is greatly reduced, I think.

The other option could be to disable commenting and push the results only to job summary (https://astroautomata.com/AirspeedVelocity.jl/stable/#Option-2:-Job-Summary).

@lostella
Copy link
Copy Markdown
Member

@hakkelt will review asap.

I agree that probably disabling comments from the benchmarking workflow, and logging the result instead, is the safest at the moment. Does the workflow fail in case of performance regressions? If so, that should be enough: red => inspect logs

@lostella lostella self-requested a review April 13, 2026 19:37
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