ShapeWorks icon indicating copy to clipboard operation
ShapeWorks copied to clipboard

ShapeWorks Optimize needs some input verification

Open akenmorris opened this issue 4 years ago • 17 comments

Many users and developers struggle with crashes in ShapeWorks Optimize step that are the result of grooming errors and other input errors. It would be nice if the optimizer did some input verification before running.

Some things that could be checked:

  1. DT inputs are actually DTs
  2. Surface on DT doesn't run off the end of the image (e.g. needs more padding)
  3. ...

Mesh : confirm mesh is manifold.

Others?

akenmorris avatar Jan 22 '21 00:01 akenmorris

Here's an example of the surface clipping the domain:

image

(in this instance, the optimizer would crash complaining about the narrow band)

medakk avatar Jan 22 '21 18:01 medakk

Thanks, @medakk , good example. We will need to add the nrrd files to our tests (once we have the checks)

akenmorris avatar Jan 22 '21 18:01 akenmorris

Thanks, @medakk , good example. We will need to add the nrrd files to our tests (once we have the checks)

I can share the files which have such clipping.

iyerkrithika21 avatar Jan 22 '21 18:01 iyerkrithika21

+optimize should check whether the provided XML file actually exists. Currently it exits with a vague error:

Could not parse XML
Inconsistency in parameters... m_domains_per_shape != m_number_of_particles.size()

medakk avatar Jan 27 '21 05:01 medakk

+if the an input filename doesn't exist, optimize should exit immediately. currently it floods the terminal with other diagnostic info and makes it hard to realize the actual problem

+if the output dir is /existing_folder/folder_that_doesnt_exist/another_folder_that_doesnt_exist/ optimize just segfaults. check for this

medakk avatar Jan 27 '21 05:01 medakk

Not sure if you have covered this scenario in the previous comments. When a list of meshes/segs/images is passed to the optimize function, it should check if the list is empty or not. Right now it crashes with the following error message :

Unzipping Data/ellipsoid_md_dist.zip into Output/ellipsoid_md_dist/
CALLING OPTIMIZATION CODE
Output/ellipsoid_md_dist/shape_models/correspondence_512_512.xml
'optimize' FAILED: basic_string::_M_construct null not valid
General exception caught.
	Returncode: 1
	Output: None

iyerkrithika21 avatar Jan 27 '21 18:01 iyerkrithika21

+if the an input filename doesn't exist, optimize should crash immediately.

I assume you mean exit, not crash? :)

akenmorris avatar Jan 27 '21 18:01 akenmorris

@iyerkrithika21 Thanks!

@akenmorris Yikes, yes. :'D

medakk avatar Jan 27 '21 18:01 medakk

  • verify if constraints create multiple (disjoint) components @HeavenlyBerserker

medakk avatar Mar 09 '21 20:03 medakk

When running a use case with segmentations/dt images, if the parameter dictionary has domain type "mesh" then the optimize command breaks with the message unable to read the file.nrrd. It would be nice to see a message that says the domain type and the input file extension are not compatible.

iyerkrithika21 avatar Jun 02 '21 04:06 iyerkrithika21

  • If the domain_type is not recognized, the error implies as issue with reading the file, not with the parameter input.
Reading inputfile: ../aligned/dod_n02_R_femur_aligned.vtk...
'optimize' FAILED: /home/runner/install/include/ITK-5.0/itkImageFileReader.hxx:136:
 Could not create IO object for reading file ../aligned/dod_n02_R_femur_aligned.vtk
  Tried to create one of the following:
    BMPImageIO
    BioRadImageIO
    Bruker2dseqImageIO
    GDCMImageIO
    GE4ImageIO
    GE5ImageIO
    GiplImageIO
    HDF5ImageIO
    JPEGImageIO
    JPEG2000ImageIO
    LSMImageIO
    MINCImageIO
    MRCImageIO
    MetaImageIO
    NiftiImageIO
    NrrdImageIO
    PNGImageIO
    StimulateImageIO
    TIFFImageIO
    VTKImageIO
  You probably failed to set a file suffix, or
    set the suffix to an unsupported type.

patkins avatar Jul 07 '21 17:07 patkins

+The error for issues with cutting plane input assumes that too few cutting plane inputs were provided, maybe the error could instead state how many were read and how many were expected. Further it does not seem to check whether the length of num_planes_per_input matches the length of cutting_planes. (Running a multi-domain model with a cutting plane, I assumed I needed to have a plane for each input, not each domain, based on the parameter name, so the naming and/or logic behind this could also be re-evaluated.)

ERROR: Incomplete cutting plane data! Number of cutting planes for every input shape is required!!
terminate called after throwing an instance of 'int'
Aborted (core dumped)

patkins avatar Jul 07 '21 17:07 patkins

+When using normals in an xml, parameters of use_xyz, mesh_based_parameters, and attribute_scales are needed in addition to use_normals, if not included shapeworks optimize with segmentation fault or not use normals without any error message.

patkins avatar Jul 16 '21 14:07 patkins

Another example, if a distance transform gets resampled incorrectly, it can become invalid. For example, we came across a DT generated by some old python/bash scripts with 0.5 isotropic spacing. Inspection of the DT showed that near the 0 level set border, values of 0.308 and then -0.692, these add up to 1.0 and indicate the the DT was generated when the spacing was 1.0. A pixel of -0.692 with spacing 0.5 is telling us the border is more than one pixel away, yet the neighboring pixel says it's 0.308 the other direction.

Running Distance Transform from Studio Groom on this correctly fixes is to values such as 0.152 and -0.349 (0.5 between them, matching the spacing).

These invalid DTs do not crash ShapeWorks, but they cause it to make nearly zero progress in optimization. The particles bounce around in place and generally can't move. Profiling shows all time is spending following vectors in the gradient image.

akenmorris avatar Nov 06 '22 22:11 akenmorris

@akenmorris Are there any actionable items for me on this issue? We can discuss this if you like.

HeavenlyBerserker avatar Jul 31 '23 18:07 HeavenlyBerserker

@HeavenlyBerserker , Karthik's comment to you when he added you was:

verify if constraints create multiple (disjoint) components @HeavenlyBerserker

I assume he means that we would check if the constraint creates unreachable islands?

I don't know if we really need to do that, but I suppose we could.

akenmorris avatar Jul 31 '23 22:07 akenmorris

Yeah, pretty sure that's what he meant. That should be easy and might be useful. I'll do that

HeavenlyBerserker avatar Aug 01 '23 01:08 HeavenlyBerserker