Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

duplicate and rename hdu rgl prio #38

Closed

Conversation

buthanoid
Copy link
Member

No description provided.

@buthanoid buthanoid mentioned this pull request Nov 4, 2021

// copying and renaming rgl_prio's hdu if it is the same of init_img
if (selectedInputImageHDU == selectedRglPrioImage) {
selectedRglPrioImage = new FitsImageHDU(selectedInputImageHDU);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use local variable to not alter the user selection = selectedRglPrioImage !

selectedRglPrioImage.setHduName(newHduName);
oifitsFile.getImageOiData().getInputParam().setRglPrio(newHduName);
}

oifitsFile.getFitsImageHDUs().add(selectedRglPrioImage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GUI and input file are now inconsistent !

@buthanoid
Copy link
Member Author

buthanoid commented Nov 4, 2021

Having a duplicate renamed HDU for RGL_PRIO must be valid in the input form, thus this HDU must be an item of the RGL_PRIO of the combo box, thus it must be present in the image library. Thus we have duplicates in the image library.

We can :

  • copy and rename the HDU just before exporting it to the algorithm
  • keep only one HDU, and correcting the file given by BSMEM to avoid non-unique HDU names. Avoiding non-unique names is done when you add images to the library, but it does not modify a result file which can thus contains non-unique hdu names.
  • keep only one HDU, and correct BSMEM algorithm. Note that the presence of duplicate HDU names in the result file does not seem to impact OImaging, since it does not pollute the library.

@bourgesl
Copy link
Member

bourgesl commented Nov 4, 2021

That's too painful. I tested and BSMEM works with duplicated HDU (same HDU_NAME), let's wait.

@bourgesl
Copy link
Member

bourgesl commented Nov 5, 2021

Finally I prefer the 2nd solution: when calling the IR software, the input OIFits structure in memory is written to a temporary file (before sending it through http) so it seems the good place to fix the generated OIFITS file to be really valid (FITS + OIFITS + Fits image validation) and the writeOIFitsFile() can pre-process / adjust the generated nom.tam Fits HDU ...

See IRModel.prepareTempFile() to fix the oifitsFile given to OIFitsWriter.writeOIFits(tmpFile.getAbsolutePath(), oifitsFile);

@buthanoid
Copy link
Member Author

It was not difficult to do it in selectHDUs since it was only testing if the HDU for RGL_PRIO was "the same" from the one for INIT_IMG.
Do you think sharing the HDU can cause problems in the rest of the software and that it is safer to do it at the very last moment (in prepareTempFile) ?
Also if we are to avoid duplicates when we send to BSMEM should we rework OIFITS file received from BSMEM to remove duplicates too ?

@bourgesl
Copy link
Member

bourgesl commented Nov 5, 2021

I am just proposing a possible solution to fix the OIFITS file (duplicate HDU + fix HDU_NAME only before writing file) that seems less complicated than fixing the all GUI to handle HDU duplicatton on the fly if INIT_IMG = RGL_PRIOR (same choice) !
To be discussed.

@bourgesl bourgesl linked an issue Nov 8, 2021 that may be closed by this pull request
@gmella
Copy link
Member

gmella commented Feb 1, 2022

Closed and maybe revisited later after a review with software authors organized by @FerreolS

@gmella gmella closed this Feb 1, 2022
@buthanoid buthanoid deleted the duplicate-hdu-init-img-rgl-prio branch February 8, 2022 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BSMEM prior image
3 participants