Skip to content

Tomography tutorials#4

Open
arthurmccray wants to merge 20 commits intomainfrom
tomography_tutorials
Open

Tomography tutorials#4
arthurmccray wants to merge 20 commits intomainfrom
tomography_tutorials

Conversation

@arthurmccray
Copy link
Collaborator

@arthurmccray arthurmccray marked this pull request as draft February 19, 2026 17:37
@arthurmccray
Copy link
Collaborator Author

arthurmccray commented Feb 19, 2026

@cedriclim1 I created this as a place to make comments on the tutorials as I'm going through the main tomo refactor PR. So far:

  • no capitol letters in directories please (/tutorials/tomography/Notebooks -> tutorials/tomography/notebooks and scripts)
  • shouldn't phantom_dataset.ipynb be something like /tomography/notebooks/tomography_00_generate_phantom.ipynb or, if there are going to be additional simulation notebooks in the future (e.g. for hyperspectral, through-focal, etc.), tomography/notebooks/simulation_notebooks/basic_phantom.ipynb
  • also the notebook for making the dataset should save the tilt_series and tilt_angles with metadata in the filenames (tilt errors and shifts so you know what the ground truth is later)
  • fourier crop vs binning... don't want to confuse people.
  • let's also move hyperparameter_optimization.ipynb to diffractive_imaging
  • move device setting to the import cell
  • add opening the tensorboard logger to a cell
  • when running tomography_inr.reconstruct there are a lot of unnecessary prints and it's missing a progress bar with estimated time left
  • tutorials should demonstrate saving/loading of the full Tomography object if possible, but at least the reconstructed volume and final alignment values (just a np.save is fine)
  • maybe clarify somewhere that the only difference between TomographyConventional and Lite is that you don't have to explicitly set the pixelated object model. And related, should the TomographyLiteConv accept the tilt_series and tilt_angles as well? That way the user doesn't have to initialize the required TomographyPixDataset
  • For the first time defining each of the schedulers in tomography_02, i would specify the various kwargs that are being set by defaults (e.g. start_factor, eta_min, T_max) and add comments of the default values
  • for conventional recons, need to specify that it's doing SIRT and the kwarg for that
  • whats up with the factor of 42 scaling difference for SIRT vs INR? is it always 42? If so it should be handled internally, if not... what's causing it?
  • generally add titles to all of your plots so it's clear what the images are (e.g. "SIRT reconstruction used for pretraining")
  • for the INR recon, some discussion of what good "standard values" are for things like num_samples_per_ray would be helpful. Are you using 100 because you're reconstructing a 100^3 volume?

@cedriclim1
Copy link
Collaborator

@arthurmccray Ready for review again, I think I addressed most of your comments:

no capitol letters in directories please (/tutorials/tomography/Notebooks -> tutorials/tomography/notebooks and scripts)

Has been addressed, see f1ebced

shouldn't phantom_dataset.ipynb be something like /tomography/notebooks/tomography_00_generate_phantom.ipynb or, if there are going to be additional simulation notebooks in the future (e.g. for hyperspectral, through-focal, etc.), tomography/notebooks/simulation_notebooks/basic_phantom.ipynb

Has been addressed, see f1ebced

also the notebook for making the dataset should save the tilt_series and tilt_angles with metadata in the filenames (tilt errors and shifts so you know what the ground truth is later)

Has been addressed, data folder includes the tilt stack and tilt angles with 1 degree rotational misalignment.

fourier crop vs binning... don't want to confuse people.

Addressed in main PR

let's also move hyperparameter_optimization.ipynb to diffractive_imaging

Moved.

I'm getting lazy, I'm just going to address the ones that need a response probably.

whats up with the factor of 42 scaling difference for SIRT vs INR? is it always 42? If so it should be handled internally, if not... what's causing it?

Need to track this down, but it is a systematic ~42 scaling factor for different types of object sizes.

@cedriclim1 cedriclim1 marked this pull request as ready for review March 3, 2026 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants