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.

Sign up for GitHub

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

Jump to bottom

Replace cspice calls with spiceql and add api accessor function #5545

Draft
wants to merge 37 commits into
base: dev
Choose a base branch
from
Draft

Replace cspice calls with spiceql and add api accessor function #5545

wants to merge 37 commits into from

Conversation

AustinSanders
Copy link
Contributor

@AustinSanders AustinSanders commented Jul 12, 2024 •
edited
Loading

Description

Sets the stage for removing cspice calls that interact directly with kernels. Uses "restincurl" to provide api access.

In order to get this merged, I'll need to:

  • Replace the remaining cspice calls
  • Write tests
  • Address inline TODOs (exception handling, etc.)
  • Edit xml/UI to include "useWeb" for appropriate applications

Related Issue

How Has This Been Validated?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation change (update to the documentation; no code change)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Infrastructure change (changes to things like CI or the build system that do not impact users)

Checklist:

  • I have read and agree to abide by the Code of Conduct
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added myself to the .zenodo.json document.
  • I have added my user impacting change to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

  • I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 5, 2024

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 5, 2024

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@AustinSanders
Replaced apollopaninit cspice calls with spiceql
8b76f4f
@AustinSanders
Updated local spice calls with searchkernels=true
5e09e3e
@AustinSanders
Replaced voyager2isis cspice calls with spiceql calls
3a3bb84
@AustinSanders
Replaced vikcal calparameters cspice calls with spiceql calls
75b0c53
@AustinSanders
Updated rosvirtis2isis cspice calls with spiceql calls
3915b69
@AustinSanders
Replaced newhorizons cspice calls with spiceql calls
aa1c94b
@AustinSanders
Replaced lronaccal cspice calls with spiceql calls
394db88
@AustinSanders
Replaced chan1m32isis cspice calls with spiceql calls
79ab710
@AustinSanders
Replaced gllssical cspice calls with spiceql calls
e563133
@AustinSanders
Replaced lrowaccal cspice calls with spiceql calls
5177066
@AustinSanders
Replaced amicacal cspice calls with spiceql calls
ead2f55
@AustinSanders
Replaced hicalconf cspice calls with spiceql calls
2a4930e
@AustinSanders
Add spiceql to environment
e2494b5
@AustinSanders
Replaced mical cspice calls with spiceql calls
b2df938
@AustinSanders
Replaced moccal cspice calls with spiceql calls
5585fe7
@AustinSanders
Replaced moclabels cspice calls with spiceql calls
b982070
@AustinSanders
Replaced ctxcal cspice calls with spiceql calls
a0ba457
@AustinSanders
Replaced hijitcube cspice calls with spiceql calls
ab42c15
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2024

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 9, 2024

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@AustinSanders AustinSanders mentioned this pull request Aug 14, 2024
Draft
@github-actions GitHub Actions
Copy link

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@github-actions GitHub Actions
Copy link

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@github-actions GitHub Actions
Copy link

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@github-actions GitHub Actions
Copy link

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5545".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

@Kelvinrr
Copy link
Collaborator

Kelvinrr commented Nov 15, 2024 •
edited
Loading

some notes:

  • There are still a lot of furnish calls, we need to remove these to allow SpiceQL to do the furnishing and instead focus on what the furnish is doing. e.g. ITime furnishes LSKs for str2et calls, just use spiceql's function and don't furnish anything. This would allow the SpiceQL interface you made whether to use local or web.
  • We can merge this without the Spice queries and handle that in another PR, not anything to do here just thinking out loud
  • Needs some API changes now that 1.0 is out, we just need to make KernelPool.load to just SpiceQL::load and remove the get instance calls.
  • SpiceQL queries are very fast, but doing some ghetto profiling we are spending most of our time in ALE, something to consider is speeding up the ale.loads speed in that SpiceQL integration ALE PR.

Kelvinrr
Kelvinrr reviewed Nov 15, 2024
View reviewed changes
isis/src/base/apps/shadow/shadow.cpp
NaifStatus::CheckErrors();

SpiceQL::KernelPool &kPool = SpiceQL::KernelPool::getInstance();
Copy link
Collaborator

Choose a reason for hiding this comment

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

These need to be removed

Kelvinrr
Kelvinrr reviewed Nov 15, 2024
View reviewed changes
isis/src/base/apps/shadow/shadow.cpp
foreach (QString kernelFile, allKernelFiles) {
kernelsUsed += kernelFile;
furnsh_c(FileName(kernelFile).expanded().toLatin1().data());
kPool.load(FileName(kernelFile).expanded().toLatin1().data());
Copy link
Collaborator

Choose a reason for hiding this comment

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

these need to change to SpiceQL::load

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kelvinrr Kelvinrr Kelvinrr left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AustinSanders @Kelvinrr