Skip to content

Update docstrings for ADMM - #2367

Open
MargaretDuff wants to merge 2 commits into
masterfrom
admm_docs
Open

MargaretDuff wants to merge 2 commits into
masterfrom
admm_docs

Conversation

@MargaretDuff

@MargaretDuff MargaretDuff commented Aug 31, 2026 •

Copy link
Copy Markdown
Member

Changes

The issue is in #2366 - updates the incorrect docs

Testing you performed

Please add any demo scripts to https://github.com/TomographicImaging/CIL-Demos/tree/main/misc

Related issues/links

Closes #2366

Checklist

  • I have performed a self-review of my code
  • I have added docstrings in line with the guidance in the developer guide
  • I have updated the relevant documentation
  • I have implemented unit tests that cover any new or modified functionality
  • CHANGELOG.md has been updated with any functionality change
  • Request review from all relevant developers

@MargaretDuff

Copy link
Copy Markdown
Member Author
image

@lauramurgatroyd lauramurgatroyd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some suggested changes
Please also update the change log

@@ -56,22 +56,16 @@ class LADMM(Algorithm):
operator: CIL Linear Operator

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please could the parameters be rearranged to match the order in the init?

\min_{x} f(x) + g(y), \text{ subject to } Ax + By = b
\min_{x,y} f(x) + g(y), \text{ subject to } Ax + By = b

In CIL, we have implemented the case where :math:`A = K`, :math:`B = -Id`, :math:`b = 0` which gives

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is Id?

Initial guess


Note

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could any similarity to PDHG still be commented on?

@lauramurgatroyd lauramurgatroyd moved this from Todo to Priority review in CIL work Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Priority review

Development

Successfully merging this pull request may close these issues.

LADMM docstring states the wrong optimisation problem (roles of f and g are swapped)

2 participants