`ImageSource.save()` desired behavior

[chris-langfield]

What should be the default and optional behaviors for the save() method?

save() calls two functions (which are currently public): save_metadata() and save_images().

Some cases we might want to cover:

• Apply transformations, save to a new starfile and new (modified) .mrcs.

• Save just a new starfile, which references the same .mrcs data. For example, representing a subset of the data

• Save only new .mrcs data while keeping the same starfile? (I don't think this would / should be done because of the possibility that metadata, e.g. CTF params, will be different after certain transformations are applied)

• Should save() require a new filename, and should the same stem be used for the saved mrcs or should that be configurable

A sketch of possible behavior (no overwrite of original starfile allowed unless the file is physically removed)

src = Source(starfile_path, max_rows = 1000)
src.save("mysource_n1000.star")  # just save truncated but identical metadata, referencing the same `.mrcs` files

src.save(starfile_path) # error: overwriting original metadata file

src.downsample(64)
src.save(starfile_path) # error: overwriting original metadata file

os.rm(starfile_path)
src.save(starfile_path) # saves the starfile, but does not write any new data (downsample is lost). references same `mrcs` files but only 1000 of them

src.save("my_source_DS64.star") # saves new images as `my_source_DS64_0_511.mrcs, my_source_DS64_512_1023.mrcs` etc

src.save("my_source_DS64.star", overwrite=True) # deletes all old `mrcs` saves new images as `my_source_DS64_0_511.mrcs, my_source_DS64_512_1023.mrcs` etc

[garrettwrong]

Thanks for writing down this discussion!

Confused about one. In this example, why would this case not write new mrcs?

src.save(starfile_path) # saves the starfile, but does not write any new data (downsample is lost). references same mrcs files but only 1000 of them

Is there any reason to think all the cases can't be accomplished by something like the following?

def save(..., overwrite=False):
   self.save_meta_data(..., overwrite)
   self.save_mrcs(..., overwrite)

[chris-langfield]

Confused about one. In this example, why would this case not write new mrcs?

src.save(starfile_path) # saves the starfile, but does not write any new data (downsample is lost). references same mrcs files but only 1000 of them

Good point.. but it probably shouldn't be with the same name as the old mrcs..

Is there any reason to think all the cases can't be accomplished by something like the following?

def save(..., overwrite=False):
   self.save_meta_data(..., overwrite)
   self.save_mrcs(..., overwrite)

Wouldn't this get rid of the possibility of saving a new starfile with the same data? I guess we can allow save_metadata() to be public for that. I don't think save_mrcs should be public because we need to be able to link the saved image data to a star file somewhere otherwise it's useless. If someone saves image data, but then changes the metadata somehow and saves the starfile separately, it might not load properly afterwards

[garrettwrong]

Good point.. but it probably shouldn't be with the same name as the old mrcs..

I guess overwrite would guard us there.

Regarding your second point. I guess that is probably fair in the context of saving a Source. If a user really wants to just save off mrcs they could do Source.images[0:512].save(fname) or something I think.

So

def save(..., overwrite=False):
   self.save_meta_data(..., overwrite)
   self._save_mrcs(..., overwrite)

?

And I think the filename stem should be user configurable. We might want to manage a save directory for the mrcs, the mate of whats called data_dir during init. Hrmm, but maybe that adds too much complexity.

@janden did you have any comments or concerns regarding behavior of Source.save? This discussion and issue came up after Chris found some strange vestigial meta save code in #732 . Thanks!

[garrettwrong]

Since we're working a lot of the preprocessing methods last release and this release, I think it might be worthwhile to revisit this topic. (I think the most relevant item is metadata). This somewhat relates to the default overwriting behavior change that was merged recently.

We can either make this into an actionable plan or if we're satisfied with how things are working already, close it after review.

[janden]

I'm trying to understand what the issue is exactly here. The concern seems to be when and how to write the corresponding mrcs file when the user asks to save a star file. Is that correct? I guess the default behavior should be to try to use the existing mrcs when possible, and if not, write a new one (maybe have a flag to override this force_new_mrcs in case the user wants to be able to separate out the image file)?

I agree that we shouldn't really consider the case of only saving the mrcs file (since this can be achieved by just calling Image.save as you pointed out.

[garrettwrong]

I gather the only remaining item of this issue left after the updates to the default save is what to do when the mrcs can truly be reused, and if it is worth the risks trying to tackle all potential of cases of that. ( I think not, but figured it might be good for Josh to work through and decide, since he might be the one doing it :D ).

My taste is that this overcomplicates things; optimizing something that isn't really a problem. I'd lean towards just always trying to save the mrcs. Today.... If user says overwrite=False and we have a dupe, fail. When overwrite=True, on dupe overwrite it. overwrite=None (auto default), I believe the existing files' mrc move over and the prior (now renamed) starfile updates internally to new mrc paths. That allows the backed up star+mrcs to be reloaded easily.

Image transformations will/should always imply a new MRCS.

That leaves talking about either slicing, or pure metadata updates that do not rely on any image details. An example of potentially safe re-use besides slicing would be assignment of estimated rotations to images, assignment of estimated translations, or the assignment CTF estimates to image data that has been processed in no other way. We do those things, but they cannot be mixed with any transformative operations otherwise the resulting star+mrcs files become incoherent.

This would have to be a large full resolution dataset to matter, at which point, we're doing operations that are probably unrealistic on a space constrained machine. We provide enough functionality that user could reuse the mrcs file if they wanted to by utilizing the public method save_metadata, or anything they can imagine with the metadata updates, instead of the high level save wrapper. Overall, the issue is that, given this can already be done with our existing code, should we try to automatically manage every case of when it can/can't in save as a feature. Sounds like a pain and treacherous to me.

I think we might be better served by adding an example that does this type of re-use save; we might have one already. I'm not sure.

[janden]

That leaves talking about either slicing, or pure metadata updates that do not rely on any image details. An example of potentially safe re-use besides slicing would be assignment of estimated rotations to images, assignment of estimated translations, or the assignment CTF estimates to image data that has been processed in no other way. We do those things, but they cannot be mixed with any transformative operations otherwise the resulting star+mrcs files become incoherent.

Agreed. There should be a way of adding estimated rotations, etc. without recreating the mrcs file.

We provide enough functionality that user could reuse the mrcs file if they wanted to by utilizing the public method save_metadata, or anything they can imagine with the metadata updates, instead of the high level save wrapper. Overall, the issue is that, given this can already be done with our existing code, should we try to automatically manage every case of when it can/can't in save as a feature. Sounds like a pain and treacherous to me.

Ok I see. Risk is that the user can call save_metadata in cases where the images have been transformed. I guess we can raise a warning in that case?