Upsample phantoms

[curtcorum]

Upsample phantoms, example: obj_ui[] = brain_phantom2D(;us=3) implemented for brain_phantom2D, brain_phantom3D and pelvis_phantom2D.

modified: Phantom.jl

[aTrotier]

Can you change

function pelvis_phantom2D(; ss=4)

to

function pelvis_phantom2D(; ss=4, us=1)

[beorostica]

Can you change

function pelvis_phantom2D(; ss=4)

to

function pelvis_phantom2D(; ss=4, us=1)

Yes, the CI test says that the variable us doesn't exists.

[cncastillo]

Hmmm as the parameters ss and us depend on the base resolution of each phantom, maybe it makes more sense to replace them by resolution_mm so it means the same for all of them (?). The sampling parameter could be calculated internally.

[curtcorum]

@aTrotier @beorostica
Good catch! I should have tested the pelvis_phantom2D...

@cncastillo

Hmmm as the parameters ss and us depend on the base resolution of each phantom, maybe it makes more sense to replace them by resolution_mm so it means the same for all of them (?). The sampling parameter could be calculated internally.

I think I read the comment differently, below original reply was to somehow calculate the needed resolution to avoid aliasing.

Having the phantom at a particular resolution is tricky as it is still locked into multiples or integer fractions of the particular source data. #319 (comment)

---

This is a good point...it is complicated though. It seems like Cartesian GRE sequences see the phantom issue the most. Radial (GRE or UTE) also sees it, sometimes even worse. Spiral and EPI with long echo trains have many less issues.

One possibility is to put a wrapper around them, if possible, as a helper function. It would take the seq file or some other explicit parameters as arguments?

I'm thinking that the simulation functions are probably the place to do the calculation, as all the needed arguments are already present and that is where the problem could occur. It could then give a warning if a possibility of undersampling exists. Also a note in the phantom and/or simulation docstrings?

The idea is to leave things as flexible as possible but giving fair warning when a phantom/simulation interaction could occur?

[curtcorum]

Would it be reasonable to include an @info output for the phantom resolution in [mm]?

Also include the phantom voxel size in the structure?

This would not break any current behavior.

[cncastillo]

Having the phantom at a particular resolution is tricky as it is still locked into multiples or integer fractions of the particular source data. #319 (comment)

Yeah, it is tricky. Let's leave it like this while we think of a better way.

Would it be reasonable to include an @info output for the phantom resolution in [mm]?

I would like to reduce the number of messages being displayed in the REPL, or at least the ones that happen when opening the UI. We could add it to the "name" of the Phantom for now. Open to suggestions on this one.

We should definitely need to report the base resolution of the Phantom in the docstring (@beorostica), so the user knows when the resampling is happening.

Also include the phantom voxel size in the structure?

That is not a bad idea, we could have a dictionary where we can put relevant information for the phantom (obj.info similar to seq.DEF). Those parameters in obj.info should be optional, and not be necessarily defined in every phantom. For example, if the sampling is not cartesian the resolution variable could not make sense.

I would revert the @info messages for now, and open an issue to add parameters to obj.info. What do you think?

[curtcorum]

@cncastillo

Having the phantom at a particular resolution is tricky as it is still locked into multiples or integer fractions of the particular source data. #319 (comment)

Yeah, it is tricky. Let's leave it like this while we think of a better way.

It could be a warning related to the Cartesian sampling matrix and phantom resolution at some point.

Would it be reasonable to include an @info output for the phantom resolution in [mm]?

I would like to reduce the number of messages being displayed in the REPL, or at least the ones that happen when opening the UI. We could add it to the "name" of the Phantom for now. Open to suggestions on this one.

We should definitely need to report the base resolution of the Phantom in the docstring (@beorostica), so the user knows when the resampling is happening.

Also include the phantom voxel size in the structure?

That is not a bad idea, we could have a dictionary where we can put relevant information for the phantom (obj.info similar to seq.DEF). Those parameters in obj.info should be optional, and not be necessarily defined in every phantom. For example, if the sampling is not cartesian the resolution variable could not make sense.

I would revert the @info messages for now, and open an issue to add parameters to obj.info. What do you think?

Sounds good! I'll revert the info message.

I'll open the obj.info structure issue as new feature, and can work on it.

[cncastillo]

I will merge this, and we can continue in #322.