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

Change spiceinit to use smithed kernels by default #3482

Closed
scsides opened this issue Nov 4, 2019 · 16 comments
Closed

Change spiceinit to use smithed kernels by default #3482

scsides opened this issue Nov 4, 2019 · 16 comments
Labels
enhancement New feature or request Missions Issues which are a priority for missions
Milestone

Comments

@scsides
Copy link
Contributor

scsides commented Nov 4, 2019

Description
By default spiceinit currently uses reconstructed quality spk and ck kernels. If the user wants to include smithed kernels, the parameters SPKSMITHED and/or CKSMITHED must be used to have spiceinit search for the respective smithed kernels.

The fearture would be to set SPKSMITHED and CKSMITHED to true instead of the current setting of false.

The result would have spiceinit use smithed kernels for an image if they were available and fall back to use reconstructed if not.

Example
spiceinit from=myimage.cub
Currently searches only for reconstructed SPKs and CKs kernels. With the new feature it would first search for smithed SPKs and CKs and then reconstructed SPKs and CKs

The feature would change the defaults for spiceinit, causing exiting pipelines and scripts that do not have SPKSMITHED/CKSMITHED explicitly set to FALSE to produce different results when smithed kernels are available.

@ascbot

This comment has been minimized.

@ascbot ascbot added the inactive Issue that has been inactive for at least 6 months label Sep 1, 2020
@jlaura
Copy link
Collaborator

jlaura commented Sep 1, 2020

@scsides I think this is a great idea. I also think that we need an RFC to push this forward because I think this could impact user pipelines if they are seeking to reproduce old work the change in default behavior could be problematic. Thoughts?

@lwellerastro
Copy link
Contributor

I think the biggest issue with this is unintentionally getting mixed spice results. Users may end up with images that have smithed kernels intermixed among images with reconstructed spice for an area of interest and not have any idea this is happening. Then wonder why (and ask us) why some images co-register and some don't. They would have to actively refer to the labels and/or the print.prt and consult the InstrumentPositionQuality and InstrumentPointingQuality keywords to see the data has different values and then understand what that means.

In the current mode, ALL images pick up reconstructed by default and there is continuity of kernel type for the working images. The user must enable picking up smithed kernels to get them and are at least aware of what is going on.

I agree, in the cases where I know smithed kernels exist and I want them applied, I wish spiceinit would just know that and I wouldn't have to remember to change settings, but this doesn't happen that often. I also see why we would want users to pick up smithed kernels by default (especially ones we generate) so that they get the best at the get go, but we don't have smithed kernels for all images in a data set and unless a user is vary aware, they are most likely to get unexpected results. Plus this is api breaking and will definitely break pipelines and scripts.

@victoronline
Copy link
Contributor

LROC is interested in this issue. Can the "Inactive" label be removed?

@jlaura
Copy link
Collaborator

jlaura commented Sep 11, 2020

@victoronline Just people commenting on this results in the inactive label being removed. We are working on figuring out how to automate that.

Can you be more specific about what you mean? From your perspective, what is the desired behavior?

@jlaura jlaura removed the inactive Issue that has been inactive for at least 6 months label Sep 11, 2020
@jessemapel
Copy link
Contributor

I think the biggest issue with this is unintentionally getting mixed spice results. Users may end up with images that have smithed kernels intermixed among images with reconstructed spice for an area of interest and not have any idea this is happening. Then wonder why (and ask us) why some images co-register and some don't. They would have to actively refer to the labels and/or the print.prt and consult the InstrumentPositionQuality and InstrumentPointingQuality keywords to see the data has different values and then understand what that means.

I agree with Lynn's concerns. Often smithed kernels are only available for a subset of the data and users could get strange results. For example, smithed kernels may only be able for one filter in a multi-filter data set. If users attempt to work with the full spectrum available, they may get strange results because one filter is using smithed data and others are using reconstructed data.

This would be a nice thing to have, but I think we need a way to handle that situation.

@jlaura jlaura added the Missions Issues which are a priority for missions label Dec 3, 2020
@jlaura
Copy link
Collaborator

jlaura commented Dec 3, 2020

Added to the mission queue based on the comment from @victoronline.

@blandoplanet
Copy link

I like this idea, but in addition to the concerns raised by @lwellerastro and @jessemapel there are a few other challenges.

  1. How do we let users know what kernels they've picked up? Or do we put the onus on the user to figure it out (I'm not in favor of this)?

  2. How do we decide which smithed kernels to use? Do we only accept/default to smithed kernels from missions? Do we accept/default to smithed kernels generated by the USGS? Do we accept/default to smithed kernels from anyone who provides them? How do we assess the quality of the kernels?

@jessemapel
Copy link
Contributor

Re mike's comments:

  1. We put in the labels and in the application logging the quality of the kernels that spiceinit loads. So, users should see if they are using predict/reconstructed/smithed kernels in their terminal/gui when they run spiceinit and then can see later on the cube labels. Building safe-guards into down-stream processing would be a lot of work and maybe not what you want to do. For example, someone creating a color mosaic could use smithed kernels which usually are only available for 1 filter/color and then control the other colors to it.
  2. From a software side, we would have to make significant changes if we want to differentiate between different qualities of smithed kernels. For right now, we would simply take all the smithed kernels that a user would get if they set spksmithed or cksmithed to true. How that set of kernels is setup is not standardized. Generally, we go off of what is archived at NAIF or what is provided by the mission team. For other smithed kernels, they are handled on a case-by-case basis. I would personally like to see an issue opened asking to have a set of smithed kernels added. From there they can be evaluated and added or not added. How to evaluate kernels is not very clear. Right now, we depend heavily upon papers and/or archival documents that are provided alongside the kernels to see how they were made. We also generally look at the coverage of the kernels and the images that were used to make them. From that a decision is made by the devs that's not really rigorous.

I view the crux of the problem being: "Is it better to have some of your data more right and some of it wrong or all of your data roughly the same amount wrong?"

If there are concerns about the quality of smithed kernels that would be picked up by a change like this, I think that's a separate issue and I would like to see specific examples before we really go into it. Smithed kernels are already fairly unusual and no two sets are created through the same process. Attempting to discuss all of that under this topic will be a huge side tangent that's not helpful for deciding what a good default functionality is. If smithed kernels are "low quality" or "dangerous" then they shouldn't even be in the kernel databases in the first place.

@rbeyer
Copy link
Contributor

rbeyer commented Dec 5, 2020

While I don't think that smithed kernels that are in the ISIS kernel database would be considered "low quality" or "dangerous," sometimes they can be ... specialized, or only applicable under certain circumstances. The problem is that while "predict" and "reconstructed" are fairly solid, understandable categories, there are really a lot of gradations in "smithed." Sometimes, smithed kernels are as robust and universally applicable as "reconstructed" kernels are. Sometimes they aren't.

I'm not sure that changing the default is a good idea here.

@jessemapel
Copy link
Contributor

Maybe we really need a better way to specify the default for spiceinit. Some type of per missions kernel set similar to meta-kernels. That way we can set the safer to use smithed kernels to default and not the more specialized kernel sets. @rbeyer remindeds me that the New Horizon's smithed kernel set is rather specialized and you can shoot yourself in the foot with it.

@github-actions
Copy link

github-actions bot commented Jun 3, 2021

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 3, 2021
@victoronline
Copy link
Contributor

I'm reaching out to my team for a comment on this issue.

@rbeyer rbeyer removed the inactive Issue that has been inactive for at least 6 months label Aug 5, 2021
@AustinSanders AustinSanders added this to the 6.1.0 milestone Aug 5, 2021
@KrisBecker
Copy link
Contributor

While smithed kernels conceptually are necessary, we have a ways to go before they are stable/reliable in ISIS to use in this setting (as the spiceinit default). It can be quite a challenge to apply smithed kernels properly because of the resources used in the production of the control network, methods used to create the ensuing kernels and requirements to precisely reconstruct the observing environment with spiceinit.

Generally, I am only considering smithed kernels created in/by the ISIS environment. There are not many, if any at all, smithed kernels produced by other sources that are commonly used in ISIS. One exception is Gaskell's SUMFILEs, which is fraught with plenty of issues to blindly rely on the applied (see sumspice) ephemeris contained therein. But that is another topic...

Infrastructure in ISIS' kernel management system explicitly supports smithed kernels. This provides some level of configuration control over how smithed kernels are used/applied in spiceinit kernel DB searches, but it will not resolve (easily or reliably) all the problems with automated smithed kernel usage.

I also agree with concerns already expressed by @lwellerastro and others regarding the uncertainties of smithed kernels as the default lookup mode in spiceinit. Here are some other concerns that should be considered:

Timing Issues

Turns out times used throughout ISIS' kernel system is complicated by things most of us are probably not aware of. Users need to have at least a basic understanding of times when evaluating kernels in ISIS, particularly smithed kernels.

spiceint uses the StartTime values in ISIS labels which are defined/assumed/required to be in the UTC system. However, kernels and times expressed in the ISIS kernels.XXXX.db files use Barycentric Dynamical Time (TDB) - TDB and ephemeris time (ET) are equivalent. TDB and UTC systems are offset by about 60 seconds (UTC < TDB) and vary based upon the date. Here is a small code snippet that demonstrates this offset:

utc = '2020-10-20T21:49:48.852'
utc_et = spice.utc2et(utc)  # Convert from UTC->ET
str_et = spice.str2et(utc)

calform = "YYYY MON DD HR:MN:SC.###### TDB ::TDB"
utc_tdb = spice.timout(utc_et, calform)  # Convert from ET->TDB

print('UTC = ', utc)
print('UET = ', utc_et)
print('SET = ', str_et)
print('TDB = ', utc_tdb)

UTC =  2020-10-20T21:49:48.852
UET =  656502658.034404
SET =  656502658.034404
TDB =  2020 OCT 20 21:50:58.034404 TDB

In the above example, UTC is about 70 seconds less than TDB.

TDB/ET is explicitly specified in the kernel DB files so a corresponding time on the ISIS cube labels is about 1 minute earlier:

  Group = Selection
    Time = ("2020 OCT 20 21:50:58.603399 TDB",
            "2020 NOV 15 00:00:00.000000 TDB")
    File = $osirisrex/kernels/spk/orx_201020_201115_201109_od294-N_postbackaway_v1.bsp
    Type = Reconstructed
  End_Group

This ISIS StartTime UTC coverage is:

tdb ='2020 OCT 20 21:50:58.603399 TDB'
tdb_et = spice.str2et(tdb)
tdb_utc = spice.et2utc(tdb_et, "ISOC", 3)

print('TDB = ', tdb)
print('TET = ', tdb_et)
print('UTC = ', tdb_utc)

TDB =  2020 OCT 20 21:50:58.603399 TDB
TET =  656502658.603399
UTC =  2020-10-20T21:49:49.421

This information can be very helpful to understand why occasionally spiceinit will report no coverage for the observation times when the ISIS label UTC times appear to fall within coverage times found in the kernels.XXXX.db files.

References:
NAIF Fundamental Concepts: See slide 11. (BTW, this is an excellent summary of all things NAIF)
Required Reading Documents: time: Naming the Seconds of Ephemeris Time

Kernel Padding

When creating CK and SPK kernels using ckwriter and spkwriter, respectively, ephemeris records are padded at the beginning of each (image) segment by an arbitrary 3 ms. That may not be enough padding and those applications should probably be modified to cover the full exposure time with a (user?) specified time to pad at each end of the segment. We have seen issues with kernel padding reported by users (e.g., #3363).

SCLK Implications

While most all ISIS camera models use the SpacecraftClockStartTime to establish observation times, spiceinit uses the StartTime UTC to select kernels. Using SCLK is also a bit of a moving target as timing is affected by spacecraft clock drift. We have seen up to 1 minute of difference in UTC times in subsequent versions of the SCLK kernels for OREX, but they stabilize over time (~3 months).

This issue likely does not affect older missions that are no longer actively updating SCLK kernels. However, when you consider that the UTC was generated using an SCLK during the mission (PDS releases are usually made 6 month after acquisition nowadays), one should expect different UTC times with new kernels still being generated/released. With that in mind, 3 ms may not be sufficient kernel padding.

References:
Required Reading Documents: sclk - SCLK rates

Light Time/Stellar Aberration Corrections

Note that any LT+S corrections are currently baked into CK and SPK kernels produced by ISIS ckwriter/spkwriter applications. Applying LT+S when using ISIS smithed kernels would be incorrect - 'NONE' needs to be specified particularly when querying the NAIF SPICE SPK kernel pool at runtime. ISIS employs a rather fragile workaround to prevent any timing corrections by placing a special character string tag in the comment of the kernels. The ISIS Spice class checks every SPK kernel for this special tag, ID:USGS_SPK_ABCORR, and sets the correction to NONE. Use of these kernels outside of ISIS will probably not work as expected without relaying this knowledge to users. And the 3 ms padding may not be enough to sync up the proper kernel. Also, in the ISIS environment, this issue only applies to the spacecraft position and not other dynamic bodies.

Valid Observation Reconstruction

There are plenty of warnings in the ckwriter/spkwriter applications regarding the proper application of the kernels produced by this process. The kernel comments contain a very detailed summary of the kernels required to be used/loaded that precisely reconstructs the observation geometry for an image. Essentially, substituting/eliminating any kernel could result in unreliable/inconsistent geometry.

We have already covered the problems with different SCLK kernels, but using other kernels will also impact observation states. For example, use of different FK or IK kernels than what were used to create the control/kernels could contain changes to frames that result in different geometry. This is also true for PCKs and SPKs for other bodies. This is particularly the case for shape models/DTMs.

This is generally less of a problem with older missions that no longer produce/release new kernels.

Shape Models and PCKs

It is required to use the same PCK and shape model/DTM that was used to create the control/kernels to precisely reconstruct the observation conditions from smithed kernels. A change in the PCK will typically result in a body orientation change which impacts geometry. At best, you should consistently use the same PCK that was used to create the shape model/DTM or you can expect different ground coordinates. Differences in PCKs will result in inconsistencies in switching geometry operations between PCKs and DTMs.

Ray tracing along the look vector from cameras to target body surface intercepts determine latitude/longitude locations and radius. The PCK is not used at all in this process. Note that not all geometry completely relies on the DTM. For example, footprintinit defaults to use the ellipsoid rather than the shape model and phase angle is computed using the ellipsoid (PCK). So if your PCK is not consistent with the DTM, you will have inconsistent geometry.

Loss of Ephemeris Data

We are seeing a lot of NAV teams include position ephemeris for the Sun, Earth and target bodies in spacecraft kernels. This allows the NAV team to more accurately preserve observing conditions throughout the mission. However, this negatively impacts the efficacy of the SPK kernels created by spkwriter because it does not propagate these ephemeris to the smithed SPKs. If the default in spiceinit is to used smithed kernels, then this will not sufficiently reproduce the observation geometry without explicit (command line) intervention. The only workaround I am aware of is to specify the spkwriter smithed SPK kernel in the spiceinit EXTRA parameter - not the SPK parameter! This has the effect to load the original SPK from the moment the image was spiceinit'ed prior to control thus providing the proper positions of other important celestial objects. Countless times I have tracked down user problems to this issue because of the confusion it causes.

spkwriter needs to be modified to include/propagate all relevant position ephemeris from the original spacecraft SPK to correct this problem.

Struct Offsets

More and more missions are using SPKs to maintain center-of-gravity (COG) models of the spacecraft over time which shift/offset payload instruments positions relative to the spacecraft frame. COG changes can be attributed to (at least) effects of spent fuel over time (there are likely other really good reasons). The Dawn, Rosetta and OREX missions are examples where struct offset SPKs are provided.

Note use of these SPKs are optional and largely do not impact observations that are far from the target body. As we have confirmed, they have significant impact on close range geometry.

We have also encountered errors when using smithed SPK kernels produced by spkwriter when these offsets are applied.

To utilize these struct offset kernels, you must add the NaifSpkCode keyword to the kernels group with the value of the NAIF instrument ID (essentially, the value of NaifFrameCode in the Kernels group) prior to spiceinit and ensure the special SPK struct offset kernel is included. You can set this up in the ISIS kernel infrastructure to load automatically, but it is only applied if you add the NaifSpkCode.

In practice, this is a problem for spkwriter because it doesn't know about the additional frame that is added to the end of the frame chain that is needed to formulate the transform to covert the position data into mission-compatible frames/codes. We are not able to precisely produce the correct state when these kernels are applied. A full assessment of the cause/impact is needed to identify and correct the problem.

Summary

Smithed kernels are a very useful product resulting from the control process. It is very difficult to produce valid smithed CK and SPK kernels in the ISIS system and more work needs to be done to make them easier to apply correctly in the ISIS environment. In its current condition, ISIS cannot reliably use them consistently without manual intervention at the command line. Most ISIS users likely do not have adequate information/knowledge to do use/apply smithed kernels without expert guidance. Granted, most older/completed missions may not encounter some of these problems because their kernel systems are no longer evolving.

Making application of smithed kernels as the default mode in spiceinit is likely to create more problems that it solves.

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

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 Feb 2, 2022
@scsides scsides removed the inactive Issue that has been inactive for at least 6 months label Feb 3, 2022
@scsides
Copy link
Contributor Author

scsides commented Feb 3, 2022

Given the concerns listed in the comments above, this change will not be implemented. The issue will be closed. If you have new information please reopen this issue and add your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Missions Issues which are a priority for missions
Projects
None yet
Development

No branches or pull requests

10 participants