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

Cil 24.2. iterative notebooks update #165

Merged
merged 11 commits into from
Oct 16, 2024
Merged

Cil 24.2. iterative notebooks update #165

merged 11 commits into from
Oct 16, 2024

Conversation

MargaretDuff
Copy link
Member

@MargaretDuff MargaretDuff commented Sep 6, 2024 •
edited
Loading

  • Removed max_iterations
  • Added partitioner to SPDHG
  • Used the right probabilities for explicit PDHG
  • Added info on TV warmstart

Closes #134 - supersedes it

@MargaretDuff MargaretDuff linked an issue Sep 6, 2024 that may be closed by this pull request
Update to CIL version 24.2 (latest release) #155
Closed
4 tasks
@MargaretDuff MargaretDuff mentioned this pull request Sep 6, 2024
Closed
@MargaretDuff MargaretDuff self-assigned this Sep 6, 2024
lauramurgatroyd
lauramurgatroyd approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@lauramurgatroyd lauramurgatroyd left a comment

Choose a reason for hiding this comment

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

Looks great. Just one spelling suggestion and one question, but ready for merge!

demos/2_Iterative/01_optimisation_gd_fista.ipynb Outdated Show resolved Hide resolved
demos/2_Iterative/01_optimisation_gd_fista.ipynb Show resolved Hide resolved
@MargaretDuff
Copy link
Member Author

MargaretDuff commented Sep 17, 2024 •
edited
Loading

@lauramurgatroyd - the default step-size didn't change, instead how we implemented the Armijio rule. It shouldn't have broken backwards compatibility... and I think it is worth raising a separate issue in CIL but I haven't had chance to debug it yet. It will work if we change the Armijio rule parameters but this is too complicated for the first iterative notebook and instead I am considering maybe a notebook on step sizes and preconditioners for GD.

@lauramurgatroyd
Copy link
Member

Okay, thanks. Yep I think we should investigate further

@MargaretDuff
Copy link
Member Author

Let's merge it in one go - there are changes in 24.1 that break backwards compatibility so we should do all at once and update the install instructions at the same time

@MargaretDuff
Copy link
Member Author

Have tested on 24.2 and all runs (although slowly so I would like to test again when the VM is free)

@MargaretDuff
Copy link
Member Author

Have tested this morning and happy to merge!

@lauramurgatroyd lauramurgatroyd changed the title Cil 24.1. iterative notebooks update Cil 24.2. iterative notebooks update Oct 16, 2024
@lauramurgatroyd lauramurgatroyd merged commit a8c7671 into TomographicImaging:main Oct 16, 2024
This was referenced Oct 16, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lauramurgatroyd lauramurgatroyd lauramurgatroyd approved these changes

Assignees

@MargaretDuff MargaretDuff

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update to CIL version 24.2 (latest release)
2 participants
@MargaretDuff @lauramurgatroyd