Workflow enhancements - #40
Conversation
| overwrite_existing: if False, the segmenter will not be run on existing | ||
| mask files, nor will existing annotations be modified even if new | ||
| mask files are generated for a timepoint. | ||
| timepoint_filter - filter for scan_experiment_dir specifying timepoints to segment |
There was a problem hiding this comment.
What's the use-case here? Specifically, how often is it expected to need to re-segment only some images from an experiment?
There was a problem hiding this comment.
An intended use case is to selectively segment (i.e. the first time) a set of images with a given segmenter. Two already used/forseeable examples:
- A user trains a classifier that is specifically accurate on a particular life stage (e.g. late larval animals) and only wants to segment those specific images with the classifier.
- Using the standard adult classifier, the user only wants to segment adult images (to prevent unnecessary runtime lost on segmenting images that don't make sense to segment).
| positions = load_data.scan_experiment_dir(experiment_root, channels=channels, timepoint_filter=timepoint_filter) | ||
| if positions: | ||
| process = segment_images.segment_positions(positions, model, mask_root, use_gpu, overwrite_existing) | ||
| if process.stderr: |
There was a problem hiding this comment.
You probably want to rethink your overall logging and error-reporting scheme. Printing warnings is pretty useless in the case where the segmenter gets run automatically by the timepoint code, and in other non-interactive uses. If it's useful to have a logfile of when segmentation succeeds, you should also add to the same logfile the failure cases.
There was a problem hiding this comment.
The use case here is not to print warnings, but to print errors. As it is now, errors (via stderr) are stashed away from the user. This lack of notification creates the scenario where the user goes forward with further processing down the pipeline without catching a potential fail at this step; later, the user has to track backwards to find that the pipeline simply did not work the way they think it did at this early step of preprocessing. The hope is to make all erroring by the segmenter transparent so that the user is aware of fails before proceeding further.
(An example of a relevant error scenario - User 1 sets up segmentation on a machine. User 2 accidentally attempts to set up segmentation while the first segementation is running, resulting in an out-of-memory error on the card for both users. User 1's failed segmentation now calculates poses and completes without any noticable error. User 1 continues trying to make measurements, etc. and realizes during analysis that a significant number of timepoints+position have no measurement data. Finally, User 1 traces this back to not having masks+poses for the relevant timepoints and is forced to run things again.)
(P.S. This problem may be partly ameliorated with the new division of labor after separating out segmentation from handling annotations.)
There was a problem hiding this comment.
Logic still applies to printed "errors" vs. "warnings". How is stdout less "stashed away" than "stderr" in non-interactive cases? Or is this for interactive use? Let's talk in person about how logging is handled, and what cases need to be addressed better. Right now, when run by the timepoint code, stdout and stderr are both redirected to the "segmentation.log" file, which is opened in append mode.
| print(f'Errors during segmentation: {process.stderr}') | ||
| with (mask_root / 'notes.txt').open('a+') as notes_file: | ||
| notes_file.write( | ||
| f'{datetime.datetime.today().strftime("%Y-%m-%dt%H%M")} These masks were segmented with model {model}\n') |
There was a problem hiding this comment.
This seems like pretty useless logging. Which masks do "these" refer to?If you want to keep a log, have the log (a) be easy to machine-parse, and (b) clearly map date/time and model file to each mask file generated. Ideally one line per mask file to make everything dead simple. (This also suggests separating out an error log from the success log.) Last, calling a logfile notes.txt is not maximally descriptive. Even segmentation_log would be more useful.
There was a problem hiding this comment.
Fair points. Obviously, better file naming and more systematic, machine-parsable content are attainable.
Currently, generating the relevant files for segmenting and actually doing the segmentation are separated over the Python/executable divide. As far as mapping date+time+model to mask files, it's not clear how to create this mapping, particularly in the case where an error causes premature stoppage of segmentation. Maybe append process.stdout? A little dirty, but would contain all the relevant information generated while running the executable. What do you think?
| the worm hatched at the first timepoint taken. | ||
| """ | ||
| for position_name, timepoint_name, image_path in load_data.flatten_positions(positions): | ||
| print(f'Annotating poses for position {position_name}') |
There was a problem hiding this comment.
What's the use-case here? Just watching to see how fast it goes? Or providing debug info when it chokes on a particular image? If primarily the latter, it would be better (especially in the context of non-interactive uses) to just make sure the position name gets kicked up as part of the exception message. I.e. catch the exception and raise a new one with the relevant information including position name. If the exceptions get properly logged (as above), then this will make debugging easier.
There was a problem hiding this comment.
Mostly the former. A reasonable example is a user running segmentation in bulk on a large number of images (which couldn't have been done during the experiment because of the sheer number of images acquired) where calculating+storing annotations takes a sufficiently long time. In this case, progress updates make it feasible to track whether the annotation is in some bad state or just taking a long time. Otherwise, the user is left to hope that this already long computation is still going and that the user isn't sinking more hours into zero gain.
2 participants
This pull request (1) adds some verbosity to aid users keep track of progress during data processing as well as during segmentation errors, and (2) exposes some additional default options for segmenting experiments. The latter incrementally builds upon the default, simplistic workflow specified for segment_experiment, but handles some obvious user cases (e.g. individual wants to segment only adult or larval images).