Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Updates to PDHG and SPDHG notebooks #134

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Updates to PDHG and SPDHG notebooks #134

wants to merge 5 commits into from

Conversation

MargaretDuff
Copy link
Member

@MargaretDuff MargaretDuff commented Aug 31, 2023 •
edited
Loading

Added partitioner to SPDHG (from #124), fixed bug in SPDHG (#128) and added info about warm start to PDHG notebooks to note change in default.

Needs to wait until release of TV warmstart into CIL.

@MargaretDuff MargaretDuff changed the title Added partitioner, fixed bug in SPDHG and added info about warmstart Updates to PDHG and SPDHG notebooks Aug 31, 2023
@MargaretDuff
Copy link
Member Author

@gfardell or @lauramurgatroyd - can you just double check this before I merge

lauramurgatroyd
lauramurgatroyd requested changes Sep 25, 2023
View reviewed changes
Copy link
Member

@lauramurgatroyd lauramurgatroyd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I have requested some changes, but also, we can't merge until we have the new release of CIL with the new warm_start parameter.

demos/2_Iterative/03_PDHG.ipynb Outdated Show resolved Hide resolved
demos/2_Iterative/04_SPDHG.ipynb Outdated Show resolved Hide resolved
@MargaretDuff
Copy link
Member Author

I have requested some changes, but also, we can't merge until we have the new release of CIL with the new warm_start parameter.

Thanks @lauramurgatroyd - that is an excellent point :)

@MargaretDuff MargaretDuff removed the request for review from gfardell September 25, 2023 08:17
@lauramurgatroyd
Copy link
Member

Hi, are we able to merge this now?

@MargaretDuff
Copy link
Member Author

I think so, warm_start got added 23.1.0 which is what we recommend currently for the notebooks

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

@lauramurgatroyd lauramurgatroyd lauramurgatroyd requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MargaretDuff @lauramurgatroyd