DOI-USGS / ISIS3

Integrated Software for Imagers and Spectrometers v3. ISIS3 is a digital image processing software package to manipulate imagery collected by current and past NASA and International planetary missions.
https://isis.astrogeology.usgs.gov
Other
197 stars 167 forks source link

Use consistent terminology, parameters, and SPICE requirements within calibration programs #1473

Closed ascbot closed 2 years ago

ascbot commented 5 years ago

Author Name: Stuart Sides (@scsides)


The calibration program within ISIS are inconsistent with regards to the parameter names used for UNIT, FLUX, OUTPUTDN, ... Some of the calibration program require spiceinit to have been run and others do not. The recommendation is to modify all of the calibration programs to use an agreed upon standard. Also, to have them not use NAIF furnish calls to get at SPICE information, but to require spiceinit to have been run prior to calibration. This would allow outside users to take advantage of the SPICE web service for calibration, presently they have to have the mission SPICE data available locally before they can run calibration programs.

scsides commented 4 years ago

Current list of calibration programs and classes that use spicelib furnsh to load kernels: isis/src/galileo/apps/gllssical/main.cpp isis/src/lro/apps/lronaccal/main.cpp isis/src/lro/apps/lronaccal/main.cpp isis/src/lro/apps/lronaccal/main.cpp isis/src/lro/apps/lronaccal/main.cpp isis/src/lro/apps/lronaccal/main.cpp isis/src/lro/apps/lrowaccal/main.cpp isis/src/lro/apps/lrowaccal/main.cpp isis/src/lro/apps/lrowaccal/main.cpp isis/src/lro/apps/lrowaccal/main.cpp isis/src/lro/apps/lrowaccal/main.cpp isis/src/mer/apps/mical/main.cpp isis/src/mer/apps/mical/main.cpp isis/src/mer/apps/mical/main.cpp isis/src/mgs/apps/moccal/main.cpp isis/src/mgs/apps/moccal/main.cpp isis/src/mgs/apps/moccal/main.cpp isis/src/mro/apps/ctxcal/main.cpp isis/src/mro/apps/ctxcal/main.cpp isis/src/mro/apps/ctxcal/main.cpp isis/src/viking/apps/vikcal/CalParameters.cpp isis/src/viking/apps/vikcal/CalParameters.cpp

isis/src/hayabusa2/apps/hyb2onccal/Hyb2OncCalUtils.h isis/src/hayabusa/apps/amicacal/AmicaCalUtils.h isis/src/messenger/apps/mdiscal/MdisCalUtils.h

isis/src/mro/objs/HiCal/HiCalConf.cpp

scsides commented 4 years ago

Related: #1790

rbeyer commented 4 years ago

I don't see hical on the list, but I know it makes SPICE calls down in the guts. For example, in https://github.com/USGS-Astrogeology/ISIS3/blob/dev/isis/src/mro/objs/HiCal/HiCalConf.cpp, which is included from hical, but not in the hical/main.cpp file, there are furnsh_c calls.

scsides commented 4 years ago

Thanks Ross, got a little tight with my grep. List has been updates

rbeyer commented 4 years ago

I figured. However, if you missed that one, there might be others that are included in a similar fashion, so not as easily detectable. Good luck!

scsides commented 4 years ago

I added all the objs in isis, and there are others, but they all look like ingestion apps

jessemapel commented 4 years ago

@Kelvinrr Here's the list of all of the other places we could read off cubes instead of furnshing kernels directly.

jessemapel commented 4 years ago

Also to capture some discussion from this morning. If we add the ability to use SPICE data already on the images from spiceinit, do we maintain the current behavior of furnshing kernels if the images haven't been spiceinit'd or do we raise an exception and require spiceinit to be run?

scsides commented 4 years ago

The list only contains the calibration related code. There are several ingestion programs that also furnish kernels, but they are a harder problem.

ascbot commented 4 years ago

I am a bot that cleans up old issues that do not have activity.

This issue has not received feedback in the last six months. I am going to add the inactive label to this issue. If this is still a pertinent issue, please add a comment or add an emoji to an existing comment.

I will post again in five months with another reminder and will close this issue on it's birthday unless it has some activity.

ascbot commented 3 years ago

I am a bot that cleans up old issues that do not have activity.

This issue has not received feedback in the last six months. I am going to add the inactive label to this issue. If this is still a pertinent issue, please add a comment or add an emoji to an existing comment.

I will post again in five months with another reminder and will close this issue on it's birthday unless it has some activity.

jessemapel commented 3 years ago

This is active still, see the lrowaccal issue linked above.

jlaura commented 3 years ago

@jessemapel The above LROWAC issue is closed and this was labelled inactive a while ago. Is this still a high priority for a fix?

scsides commented 3 years ago

It is still import to reduce some of the tech debt, and required to allow the calibration programs to work with direct access to the SPICE files.

kberryUSGS commented 3 years ago

This issue (except for the parameter name standardization) will need to be fixed as part of the IAA project this year to get the calibration programs working without local spice data. Any ideas on how to mark this as an issue that will need to be fixed as part of a funded, but not yet scheduled project?

victoronline commented 3 years ago

This is still active.

jessemapel commented 3 years ago

@victoronline We have a specific funded deliverable to do this, so it won't be scheduled in the support meeting today but we will internally schedule it to be worked sometime this fiscal year.

github-actions[bot] commented 3 years ago

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.

scsides commented 3 years ago

We have more of these to go

jessemapel commented 3 years ago

@scsides Do we know exactly which apps need this still?

kberryUSGS commented 3 years ago

@scsides, @jessemapel: of Stuart's original list, mical is the only calibration app that still has furnsh calls. It was unable to be updated during the ISIS Calibration Sprint because it isn't actually possible to spiceinit a MER cube. Without being able to spiceinit the cube, there isn't really any way I'm aware of to get around needing local spice kernels. At least all the kernels required by mical are the in base kernels area, and not a mission area.

kberryUSGS commented 3 years ago

To follow on to this, though it might belong in a new ticket, there are still furnsh calls in the following non-calibration apps. I've listed the apps here and some information about why there are furnsh calls and what is getting furnished:

rbeyer commented 3 years ago

FYI, I do not think that hicrop is part of the HiRISE Team processing pipelines, but I suppose that doesn't make a great deal of difference in this context.

github-actions[bot] commented 2 years ago

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.