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.

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

metrics and docs #25

Merged
merged 3 commits into from
Jun 28, 2024
Merged

metrics and docs #25

merged 3 commits into from
Jun 28, 2024

Conversation

casperdcl
Copy link
Member

@casperdcl casperdcl commented Jun 28, 2024

@casperdcl casperdcl self-assigned this Jun 28, 2024
@casperdcl casperdcl added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 28, 2024
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.

Looks good, but I haven't checked in a lot of detail, but go ahead.

I just have some comments on naming etc, to avoid confusing participants

python
docker run --rm -it -v /path/to/data:/mnt/share/petric:ro -v .:/workdir -w /workdir synerbi/sirf:edge-gpu /bin/bash
# ... or ideally synerbi/sirf:latest-gpu after the next SIRF release!
pip install git+https://github.com/TomographicImaging/Hackathon-000-Stochastic-QualityMetrics
Copy link
Member

Choose a reason for hiding this comment

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

Why not add this to our docker image? But this is fine of course

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it'll change a lot right now, and eventually be baked in to CIL itself, so wouldn't add it to the SIRF image for now.

petric.py Outdated
TensorBoard(logdir=outdir, transverse_slice=transverse_slice, coronal_slice=coronal_slice)]
(tb_cbk := TensorBoard(logdir=outdir, transverse_slice=transverse_slice, coronal_slice=coronal_slice))]

if ground_truth:
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to rename ground_truth to converged_image

Copy link
Member Author

Choose a reason for hiding this comment

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

or reference?

petric.py Show resolved Hide resolved
@KrisThielemans
Copy link
Member

Looks like I cannot review again via the app. But it's good to go!

@casperdcl casperdcl merged commit 5483ba6 into main Jun 28, 2024
2 checks passed
@casperdcl casperdcl deleted the metrics-and-docs branch June 28, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants