JMMC-OpenDev / OI-Imaging-JRA

Design and specification of an interface to image reconstruction and model fitting from optical interferometric data
4 stars 6 forks source link

Keeping track of previous reconstruction #15

Closed FerreolS closed 2 years ago

FerreolS commented 2 years ago

When doing several reconstructions successively on the same OIfits file. It can be nice to keep track of all the previous reconstruction. This can be done easily by adding a new HDU containing the current initial image and place the result in the primary. This may lead to several issues:

GillesDuvert commented 2 years ago

I thought 'keeping track" is something the GUI should manage? If the input parameter list contains NUMBER_OF_RECONSTRUCTION_SO_FAR=33 Then it can be incremented to 34 upon return of the imaging software, or something similar?

FerreolS commented 2 years ago

It is something that can be handled by the GUI using a session description that has still to be defined. However, reading the 3.2 section of OI-Interface.pdf . I though that it can be also something that can be handled easily by software (with the GUI patching software that does do it). In any case, if we decide to stack previous reconstruction result in a single OIFits file, we will face the same two questions. I think that storing the OUTPUT/INPUT PARAM keyword in each image HDU header is OK and easy. But we may have to change HDU names to prevent having several OUTPUT50 name.

FerreolS commented 2 years ago

The solution can be either defining the name with adding iteration number since the beginning or incrementing a new counter such the name become OUTPUT_k_50 for the k-th image reconstructed with 50 iteration. The only difference I see is if the number of iteration is zero then we will still have duplicate number in the first case. However the image should be identical in that case (but not necessarily as some constraint may have been enforced (like normalisation) before any iteration) .

jsy1001 commented 2 years ago

I'm wary of storing input/output parameters in the headers of image HDUs. We originally decided not to do this in case any future algorithm needs parameters that can't straightforwardly be stored as header cards (e.g. vectors or matrices).

GillesDuvert commented 2 years ago

I strongly advocate a clear separation of history memory btw. the caller (GUI) and the softwares. They just need to copy back all the input tables with their headers (and they do it now). What the 'outer world' (=OImaging) does with those headers, keywords, history etc is not our concern. Let's image reconstruction software authors stay focussed on their expertise...

FerreolS commented 2 years ago

I'm not sure to understand. What is the right way to do if the user want to keep track of previous reconstruction:

What is your preference? (Mine is on the second solution that seems a bit simpler to manage at the GUI level.)

emmt commented 2 years ago

For me, initial parameters, the selection of data, and the algorithm should (hopegully) uniquely define the result. Intermediate images are useless unless you want to keep the user attention. So it is rather the job of the GUI. It may be a not so good idea to save them in the output file. It may give the impression that "early stopping" results are worth to consider as in the good old times of the RL method ;-)

FerreolS commented 2 years ago

From section 3.2, I understand that the mandatory images are Init image and final image that are identified with their HDUNAME in the keywords INIT_IMG and LAST_IMG respectively. The final image being in the primary HDU. But there is nothing about images in other HDUs that can contains priors or anything needed by the user.
What I want to ensure are:

I just to make these 2 points fixed, approved by everyone and written it in the OI-Interface document

FerreolS commented 2 years ago

I can make two different issues for the 2 points above if you prefer as the subject differ now for the title

emmt commented 2 years ago

If software blindly copies input HDU's in the output, then if input already has images at different iterates, all iterates will be mixed. So why not having a different output for the intermediate images? For instance, software can save into another FITS file the current iterate every, say n, iterations and the GUI can track this file (or these files) to display a live image. However, there is a pending issue: we need a locking mechanism to avoid overwritting images. This can be just the file name which is numbered and the GUI can do the necessary cleanup of these temporary files.

FerreolS commented 2 years ago

I agree that tracking reconstruction history must be done at OImaging level (it can be just history but it can be more complex e.g. if we make reconstruction decreasing some parameter a each run). Up to now we didn't chose the mechanism to track this: it will be probably a kind of archive storing everything in multiple files but it can also be a single OIfits file with many hdu. As I'm not sure we should settle this mechanism now (it has many implications especially if we want to store it in a database), I would prefer to let the two options possible. So I would like to ensure that all hdus unused by sofwares are all blindly copied. On my opinion, softwares should just write OUTPUT PARAM table, the final image and the model as stated in #12 without destroying other HDUs. This will let everything open for future features of OImaging without bothering you again.

But we still have to find a mechanism preventing HDUNAME duplicates

jsy1001 commented 2 years ago

FYI BSMEM rewrites the HDUs containing the input data (OI_VIS2 and OI_T3) in order to add the model values, as proposed in the specification document. It wouldn't be too difficult to implement copying of other HDUs provided we can define a convention to prevent HDUNAME clashes.

You might be interested to read about the grouping convention here: https://fits.gsfc.nasa.gov/registry/grouping/grouping.pdf

GillesDuvert commented 2 years ago

I vote for closing this and open a new issue "How to prevent HDUNAME duplicates"

FerreolS commented 2 years ago

I have split the two points in two issue: