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

add metrics as per wiki #46

Merged
merged 4 commits into from
Jul 10, 2024
Merged

add metrics as per wiki #46

merged 4 commits into from
Jul 10, 2024

Conversation

casperdcl
Copy link
Member

@casperdcl casperdcl commented Jul 9, 2024 •
edited
Loading

@casperdcl casperdcl self-assigned this Jul 9, 2024
@casperdcl casperdcl added the enhancement New feature or request label Jul 9, 2024
casperdcl
casperdcl commented Jul 9, 2024
View reviewed changes
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
KrisThielemans
KrisThielemans requested changes Jul 9, 2024
View reviewed changes
Copy link
Member

@KrisThielemans KrisThielemans left a comment

Choose a reason for hiding this comment

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

mostly naming, but some conceptual changes as well as code questions.

petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
petric.py Outdated Show resolved Hide resolved
@casperdcl casperdcl force-pushed the metrics branch 3 times, most recently from 9e38e01 to e15033a Compare July 9, 2024 18:33
casperdcl
casperdcl commented Jul 9, 2024
View reviewed changes
petric.py Outdated Show resolved Hide resolved
KrisThielemans
KrisThielemans requested changes Jul 10, 2024
View reviewed changes
petric.py Show resolved Hide resolved
petric.py Show resolved Hide resolved
@KrisThielemans
Copy link
Member

One question: I'd like to be able to call this callback on existing images, without running a recon. As the callbacks rely on algorithm.x, algorithm.iteration and (currently) algorithm.update_objective_interval, this is currently awkward. What about moving the calculation bits of __call__ into a compute(image) function (return a tuple of values I guess), and let __call__ handle the interval and TensorBoard?

KrisThielemans
KrisThielemans requested changes Jul 10, 2024
View reviewed changes
Copy link
Member

@KrisThielemans KrisThielemans left a comment

Choose a reason for hiding this comment

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

location moved.

Want to split __call__ in this PR or a separate one? It'll be really useful for me...

petric.py Outdated Show resolved Hide resolved
@casperdcl casperdcl merged commit beb42e9 into main Jul 10, 2024
2 checks passed
@casperdcl casperdcl deleted the metrics branch July 10, 2024 15:57
@casperdcl
Copy link
Member Author

moved __call__ request to #51

@casperdcl casperdcl mentioned this pull request Aug 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epapoutsellis epapoutsellis epapoutsellis left review comments

@KrisThielemans KrisThielemans KrisThielemans requested changes

@paskino paskino Awaiting requested review from paskino

Assignees

@casperdcl casperdcl

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@casperdcl @KrisThielemans @epapoutsellis