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

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

Closed
ascbot opened this issue Feb 12, 2019 · 24 comments
Labels
enhancement New feature or request inactive Issue that has been inactive for at least 6 months

Comments

@ascbot
Copy link
Contributor

ascbot commented Feb 12, 2019


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.

@ascbot ascbot added the enhancement New feature or request label Feb 12, 2019
@scsides
Copy link
Contributor

scsides commented Dec 16, 2019

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
Copy link
Contributor

scsides commented Dec 16, 2019

Related: #1790

@rbeyer
Copy link
Contributor

rbeyer commented Dec 16, 2019

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
Copy link
Contributor

scsides commented Dec 16, 2019

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

@rbeyer
Copy link
Contributor

rbeyer commented Dec 16, 2019

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
Copy link
Contributor

scsides commented Dec 16, 2019

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

@jessemapel
Copy link
Contributor

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

@jessemapel
Copy link
Contributor

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
Copy link
Contributor

scsides commented Dec 17, 2019

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

@ascbot
Copy link
Contributor Author

ascbot commented Sep 1, 2020

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 ascbot added the inactive Issue that has been inactive for at least 6 months label Sep 1, 2020
@jessemapel jessemapel removed the inactive Issue that has been inactive for at least 6 months label Oct 15, 2020
@ascbot
Copy link
Contributor Author

ascbot commented Oct 16, 2020

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 ascbot added the inactive Issue that has been inactive for at least 6 months label Oct 16, 2020
@jessemapel jessemapel removed the inactive Issue that has been inactive for at least 6 months label Oct 16, 2020
@jessemapel
Copy link
Contributor

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

@jlaura
Copy link
Collaborator

jlaura commented Dec 3, 2020

@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
Copy link
Contributor

scsides commented Dec 3, 2020

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.

@krlberry
Copy link
Contributor

krlberry commented Dec 3, 2020

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
Copy link
Contributor

This is still active.

@jessemapel
Copy link
Contributor

@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
Copy link

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.

@github-actions github-actions bot added the inactive Issue that has been inactive for at least 6 months label Jun 14, 2021
@scsides scsides removed the inactive Issue that has been inactive for at least 6 months label Jun 15, 2021
@scsides
Copy link
Contributor

scsides commented Jun 15, 2021

We have more of these to go

@jessemapel
Copy link
Contributor

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

@krlberry
Copy link
Contributor

@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.

@krlberry
Copy link
Contributor

krlberry commented Jun 17, 2021

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:

  • apollo/apps/apollopaninit/main.cpp
    Apollo version of spiceinit – won’t be possible to remove furnsh call.

  • chandrayaan1/apps/chan1m32isis/chan1m32isis.cpp
    This is an ingestion app. It makes fairly complex calculations based on the Start time and timing tables supplied along with the PDS image. It also uses the times to add lines that were dropped during transmission/processing.

  • mro/apps/hicrop/hicrop.cpp
    This one is a specialized app, it uses the gaps in the CK to identify where the hirise cube should be cropped. Even small gaps cause big problems for hirise. SCLK and LSK are used with the CK

  • newhorizons/apps/mvic2isis/mvic2isis.cpp (furnishes lsk and sclk)

  • rosetta/apps/rosvirtis2isis/main.cpp (furnishes lsk and sclk)

  • system/apps/kerneldbgen/SpiceDbGen.cpp (This will always require kernels given that it's using local kernels to generate the kernel database files the spiceinit uses.)

  • voyager/apps/voy2isis/main.cpp (furnishes lsk and sclk)

@rbeyer
Copy link
Contributor

rbeyer commented Jun 17, 2021

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.

@rbeyer rbeyer closed this as completed Jun 17, 2021
@rbeyer rbeyer reopened this Jun 17, 2021
@github-actions
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request inactive Issue that has been inactive for at least 6 months
Projects
None yet
Development

No branches or pull requests

7 participants