Foundational SPICE tools #279

laspsandoval · 2023-11-15T18:31:27Z

Change Summary

Overview

-Added some basic tools.

New Dependencies

None

New Files

spice_utils.py
- loads and clears kernels
- lists loaded kernel filenames
- lists all constants in the Spice constant pool
- using historical attitude kernels, figures out processing dates

Testing

test_spice_utils.py
- tests spice_utils.py
I put almost all of the ultra spice test data in the /tools/tests/test_data/spice directory. Github would not let me put de440.bsp in because the file was too large. We should discuss how we want to handle that.

Note: there are other test files in: smb://lasp-store.lasp.colorado.edu/projects/Phase_Development/IMAP-SOC/Technical/POC/MOC Interface/PRELIM_SPICE_TEST1.zip that I have not yet added to the repo.

closes #66
closes IMAP-Science-Operations-Center/sds-data-manager#82

laspsandoval · 2023-11-15T18:56:23Z

pre-commit.ci autofix

tech3371 · 2023-11-15T21:32:33Z

tools/spice/spice_utils.py

+
+    def furnsh(self):
+        """Load a kernel file into the SPICE system."""
+        self.clear()


What does this do? does it clear spice kernel pool?

If you could add a note about why it's important to clear spice kernel pool, it will help when someone new reads it.

tech3371 · 2023-11-15T21:36:40Z

tools/spice/spice_utils.py

+        # 0 : starting from first match
+        # 1000 : num variable names retrieved in call
+        # 81 : max len of variable name
+        kervars = spice.gnpool("*", 0, 1000, 81)


can you change kervars to kernel_vars?

tech3371 · 2023-11-15T21:48:43Z

tools/spice/spice_utils.py

+        # retrieve names of kernel variables
+        # * means matches all variables
+        # 0 : starting from first match
+        # 1000 : num variable names retrieved in call
+        # 81 : max len of variable name
+        kervars = spice.gnpool("*", 0, 1000, 81)


We could include the link in the comment too if someone like to dive deeper.

Suggested change

# retrieve names of kernel variables

# * means matches all variables

# 0 : starting from first match

# 1000 : num variable names retrieved in call

# 81 : max len of variable name

kervars = spice.gnpool("*", 0, 1000, 81)

# retrieve names of kernel variables using below inputs per https://naif.jpl.nasa.gov/pub/naif/toolkit_docs/C/cspice/gdpool_c.html

# name = '*', means name of the variable whose value is to be returned.

# start = 0, Which component to start retrieving for `name'.

# room = 1000, The largest number of values to return.

# n = 81, Number of values returned for `name'.

tech3371 · 2023-11-15T21:50:13Z

tools/spice/spice_utils.py

+    return result
+
+
+def ls_attitude_coverage(custom_pattern=None) -> tuple:


just to be sure ls here means list, right?

That's right

looks like Greg made suggestion above to spell it out. can we do same here? ty!

tech3371 · 2023-11-15T21:55:18Z

tools/spice/spice_utils.py

+
+        matching_pattern = re.match(pattern, basename)
+        if matching_pattern:
+            parts = matching_pattern.groups()


a comment here would be helpful.

tech3371 · 2023-11-15T21:55:37Z

tools/spice/spice_utils.py

+
+        # Historical attitude kernels only
+        if custom_pattern is None:
+            pattern = (


a comment would be helpful

tech3371 · 2023-11-15T21:57:26Z

tools/tests/unit/test_spice_utils.py

+    n_files_before_clear = spice.ktotal("ALL")
+    assert n_files_before_clear > 0
+
+    # Clear loaded kernels and verify


These comments are helpful. ty!

tech3371 · 2023-11-15T21:58:43Z

tools/tests/unit/test_spice_utils.py

+    assert n_files_loaded_after_clear == 0
+
+
+def test_furnsh_type(caplog):


what's caplog? just curious

greglucas

My main comment would be that this seems like it'd be nice to have your kernel manager be a contextmanager (you already have manager in the name even ;) ). Let me know if you have thoughts for/against that or any questions and I'd be happy to chat.

The other question I have since I haven't tried this PR out locally is with the spice files you uploaded and whether they would even work for me because they have some hard-coded paths associated with them. Will this cause issues for people in the future?

greglucas · 2023-11-16T22:15:47Z

tools/tests/test_data/spice/imap_static_kernels.tm

+   PATH_SYMBOLS += ( 'GENERIC' )
+   PATH_VALUES  += ( '/project/mini-rf/poc/LRO/data/storage/ancillary/spice/frames' )
+
+   PATH_SYMBOLS += ( 'IMAP' )
+   PATH_VALUES  += ( '/project/sis/users/duttont1/software/imap/lib' )


This doesn't seem like they'd work? Should we change them to be relative to the project somehow, like: /mnt/spice/... Or can they be removed altogether?

greglucas · 2023-11-16T22:16:56Z

tools/tests/test_data/spice/imap_static_kernels_euler.tm

@@ -0,0 +1,17 @@
+KPL/MK
+
+Meta kernel that loads the arecibo configuration kernels into the sarbeta program.


Suggested change

Meta kernel that loads the arecibo configuration kernels into the sarbeta program.

Meta kernel that loads the IMAP configuration kernels.

I was wondering if arecibo was some sort of spice-specific buzz word or the actual arecibo telescope 😅

greglucas · 2023-11-16T22:17:14Z

tools/tests/test_data/spice/imap_static_kernels_euler.tm

+\begindata
+
+   PATH_SYMBOLS += ( 'IMAP' )
+   PATH_VALUES  += ( '/Users/lasa6858/Desktop/ultra/ultra_prototype_v1/kernels' )


Won't work for other users, make more general?

greglucas · 2023-11-16T22:29:42Z

tools/spice/spice_utils.py

+logger = logging.getLogger(__name__)
+
+
+class SpiceKernelManager:


This seems like it'd be nice to be a context manager, so the loading/closing of the kernels was all taken care of within the scope of when you're using it. Then a user wouldn't need to call furnsh() or clear() directly. Thoughts on that?

Looks like this might even already be implemented in spiceypy? So, do we need our own version here or can you expose that somehow instead.
https://github.com/AndrewAnnex/SpiceyPy/blob/22f7aaa0944d01994540f0acd5136a54ed239efe/src/spiceypy/spiceypy.py#L289
https://spiceypy.readthedocs.io/en/main/contextmanager.html

greglucas · 2023-11-16T22:32:19Z

tools/spice/spice_utils.py

+    extensions : list of str, optional
+        A list of file extensions to be considered as valid SPICE kernel files.


Additional extensions? If I add one entry .greg to the extension list, I wouldn't necessarily want to have to add all the other default extensions too.

Are there other extensions we need to consider even, or can we just only allow default ones?

We could only allow the default ones. I think it's useful during development to be able to select a single extension that I want to furnish and then view the constants for only that kernel type. But this could totally be used without passing the extensions and just relying on the default ones.

greglucas · 2023-11-16T22:46:23Z

tools/spice/spice_utils.py

+    result = {}
+    for kervar in sorted(kervars):
+        # retrieve data about a kernel variable
+        n, kernel_type = spice.dtpool(kervar)  # pylint: disable=W0632


I had to look up W0632, can you balance the tuple instead of ignoring the linter?

greglucas · 2023-11-16T22:51:30Z

tools/spice/spice_utils.py

+
+
+def ls_attitude_coverage(custom_pattern=None) -> tuple:
+    """Process attitude kernels to extract and convert dates to UTC format.


I'm not following what this function is being used for. Do we need a custom pattern, or can we prescribe the pattern directly? I think you might be trying to be too nice and flexible, but that can come later if we need to add it.

Maybe more basic is why we are getting this data out of the filename instead of the kernel data itself? i.e. can we query the object within spicepy somehow to get this information instead of parsing filenames.

This might change. It was my first attempt at figuring out how we would select the most recent historical attitude kernel. I will put a TODO here in case we find that symlink can do that. I don't want to delete it quite yet though.

greglucas · 2023-11-16T22:55:14Z

tools/tests/unit/test_spice_utils.py

+
+    expected = [str(directory / "imap_spk_demo.bsp")]
+
+    assert sorted(result) == sorted(expected)


They are length-1 lists, so don't need to sort.

greglucas · 2023-11-16T22:56:16Z

tools/tests/unit/test_spice_utils.py

+
+    # Test with valid extensions
+    result = ls_attitude_coverage()
+    assert result is not None


You are doing an isinstance right below this, so that will also ensure result is not None.

Suggested change

assert result is not None

greglucas · 2023-11-16T22:56:35Z

tools/tests/unit/test_spice_utils.py

+
+    # Test with an empty directory
+    kernel_object.clear()
+    empty_result = ls_attitude_coverage()


Should this raise or be an empty tuple instead?

sdhoyt · 2023-11-20T19:53:33Z

tools/spice/spice_utils.py

+        if extensions is None:
+            result.append(file)
+        # Check if the file ends with any of the specified extensions
+        elif any(file.endswith(ext) for ext in extensions):


Could this filtering be done using a file type input for ktotal() and kdata() instead of passing in "ALL"?

There is definitely more than one way to do this. kdata gives information about the ith kernels loaded of a certain type. Instead of using extensions I could use kernel type (SPK instead of .bsp for example). I started using extensions in SpiceKernelManager and just continued using it in list_loaded_kernels. Would it be more helpful for you if I added a way to use kernel type instead?

bourque

Awesome work here, this is super useful! I just had mostly nit-picky naming and formatting suggestions.

bourque · 2023-11-20T20:55:06Z

tools/spice/spice_utils.py

+
+    Parameters
+    ----------
+    extensions: list of str, optional


Suggested change

extensions: list of str, optional

extensions: list of str or None, optional

bourque · 2023-11-20T20:56:30Z

tools/spice/spice_utils.py

+    return result
+
+
+def ls_spice_constants() -> dict:


Suggested change

def ls_spice_constants() -> dict:

def list_all_constants() -> dict:

bourque · 2023-11-20T20:58:17Z

tools/spice/spice_utils.py

+    : tuple
+        Tuple giving the most recent start and end time in ET.
+    """
+    att_kernels = ls_kernels([".ah.bc", ".ah.a"])


Suggested change

att_kernels = ls_kernels([".ah.bc", ".ah.a"])

attitude_kernels = ls_kernels([".ah.bc", ".ah.a"])

bourque · 2023-11-20T21:00:14Z

tools/tests/test_data/spice/imap_static_kernels_euler.tm

@@ -0,0 +1,17 @@
+KPL/MK
+
+Meta kernel that loads the arecibo configuration kernels into the sarbeta program.


I was wondering if arecibo was some sort of spice-specific buzz word or the actual arecibo telescope 😅

bourque · 2023-11-20T21:00:52Z

tools/tests/test_data/spice/imap_static_kernels.tm

@@ -0,0 +1,30 @@
+KPL/MK
+
+Meta kernel that loads the arecibo configuration kernels into the sarbeta program.


Suggested change

Meta kernel that loads the arecibo configuration kernels into the sarbeta program.

Meta kernel that loads the IMAP configuration kernels.

bourque · 2023-11-20T21:05:11Z

tools/tests/unit/test_spice_utils.py

+    assert n_files_loaded == n_test_files
+
+
+def test_clear(kernel_object):


Suggested change

def test_clear(kernel_object):

def test_clear(kernel_object):

"""Tests the clearing of kernels via the ``clear`` method"""

bourque · 2023-11-20T21:07:26Z

tools/tests/unit/test_spice_utils.py

+    assert n_files_loaded_after_clear == 0
+
+
+def test_furnsh_type(caplog):


I'm a bit confused as to what this test is checking for. It looks more like it is making sure that an invalid kernel type is resulting in an expected error message? If so, might want to rename the test to something like test_invalid_kernel. It would also be helpful to include a docstring.

bourque · 2023-11-20T21:08:00Z

tools/tests/unit/test_spice_utils.py

+    kernel_object.clear()
+
+
+def test_ls_kernels(kernel_object):


Suggested change

def test_ls_kernels(kernel_object):

def test_ls_kernels(kernel_object):

"""Tests the ``ls_kernels`` function"""

bourque · 2023-11-20T21:09:38Z

tools/tests/unit/test_spice_utils.py

+    assert sorted(result) == sorted(expected)
+
+
+def test_ls_spice_constants(kernel_object):


Suggested change

def test_ls_spice_constants(kernel_object):

def test_ls_spice_constants(kernel_object):

"""Tests the ``ls_spice_constants`` function"""

bourque · 2023-11-20T21:10:41Z

tools/tests/unit/test_spice_utils.py

+    assert list(result.keys()) == expected_keys
+
+
+def test_ls_attitude_coverage(kernel_object):


Suggested change

def test_ls_attitude_coverage(kernel_object):

def test_ls_attitude_coverage(kernel_object):

"""Tests the ``ls_attitude_coverage`` function"""

tech3371

It looks good to me. I had one comment.

tech3371 · 2023-12-13T16:34:46Z

imap_processing/ialirt/tests/unit/test_decom_ialirt.py

Was this brought in by mistake?

for more information, see https://pre-commit.ci

* add spice tools

laspsandoval added this to the SPICE Tools Complete milestone Nov 15, 2023

laspsandoval self-assigned this Nov 15, 2023

laspsandoval requested review from greglucas, tech3371, bryan-harter, bourque, sdhoyt, GFMoraga and maxinelasp November 15, 2023 19:00

tech3371 reviewed Nov 15, 2023

View reviewed changes

greglucas reviewed Nov 16, 2023

View reviewed changes

sdhoyt reviewed Nov 20, 2023

View reviewed changes

bourque reviewed Nov 20, 2023

View reviewed changes

laspsandoval requested a review from KenWFairchild November 22, 2023 18:43

laspsandoval force-pushed the dev branch from 5060c14 to 4263c92 Compare December 11, 2023 22:36

tech3371 approved these changes Dec 13, 2023

View reviewed changes

imap_processing/ialirt/tests/unit/test_decom_ialirt.py Outdated

Copy link

Contributor

tech3371 Dec 13, 2023

Choose a reason for hiding this comment

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

Was this brought in by mistake?

laspsandoval added 13 commits December 13, 2023 13:46

adding ialirt

3c926b4

updates

f654feb

adding hit decom

9908711

adding other instruments

64b9b15

update

748b1f6

formatting

bd45811

mistaken changes

eb84eb3

minor updates

a47d768

response to PR

74530ab

appease codecov

2f7fd1e

spice utils

7f690cd

removed large file

30018f3

formatting

1c21749

laspsandoval and others added 7 commits December 13, 2023 13:46

formatting

34e860a

update

b7fa7dc

kernels

5f7dd96

black fix

bfdab01

[pre-commit.ci] auto fixes from pre-commit.com hooks

4f248a7

for more information, see https://pre-commit.ci

pr response

cf45b18

minor updates

a212361

laspsandoval force-pushed the dev branch from 6fcd433 to a212361 Compare December 13, 2023 20:47

laspsandoval merged commit be2f45e into IMAP-Science-Operations-Center:dev Dec 13, 2023

sdhoyt linked an issue Dec 14, 2023 that may be closed by this pull request

Create SPICE libraries for processing #66

Closed

laspsandoval added a commit to laspsandoval/imap_processing that referenced this pull request Apr 2, 2024

Foundational SPICE tools (IMAP-Science-Operations-Center#279)

dedc30f

* add spice tools

		return result


		def ls_attitude_coverage(custom_pattern=None) -> tuple:

		assert n_files_loaded_after_clear == 0


		def test_furnsh_type(caplog):

		@@ -0,0 +1,17 @@
		KPL/MK

		Meta kernel that loads the arecibo configuration kernels into the sarbeta program.

	Meta kernel that loads the arecibo configuration kernels into the sarbeta program.
	Meta kernel that loads the IMAP configuration kernels.

		logger = logging.getLogger(__name__)


		class SpiceKernelManager:

		extensions : list of str, optional
		A list of file extensions to be considered as valid SPICE kernel files.



		def ls_attitude_coverage(custom_pattern=None) -> tuple:
		"""Process attitude kernels to extract and convert dates to UTC format.


		expected = [str(directory / "imap_spk_demo.bsp")]

		assert sorted(result) == sorted(expected)

	extensions: list of str, optional
	extensions: list of str or None, optional

	def ls_spice_constants() -> dict:
	def list_all_constants() -> dict:

	att_kernels = ls_kernels([".ah.bc", ".ah.a"])
	attitude_kernels = ls_kernels([".ah.bc", ".ah.a"])

		@@ -0,0 +1,30 @@
		KPL/MK

		Meta kernel that loads the arecibo configuration kernels into the sarbeta program.

		assert n_files_loaded == n_test_files


		def test_clear(kernel_object):

	def test_clear(kernel_object):
	def test_clear(kernel_object):
	"""Tests the clearing of kernels via the ``clear`` method"""

	def test_ls_kernels(kernel_object):
	def test_ls_kernels(kernel_object):
	"""Tests the ``ls_kernels`` function"""

		assert sorted(result) == sorted(expected)


		def test_ls_spice_constants(kernel_object):

	def test_ls_spice_constants(kernel_object):
	def test_ls_spice_constants(kernel_object):
	"""Tests the ``ls_spice_constants`` function"""

		assert list(result.keys()) == expected_keys


		def test_ls_attitude_coverage(kernel_object):

	def test_ls_attitude_coverage(kernel_object):
	def test_ls_attitude_coverage(kernel_object):
	"""Tests the ``ls_attitude_coverage`` function"""

Foundational SPICE tools #279

Foundational SPICE tools #279

Conversation

laspsandoval commented Nov 15, 2023 • edited by sdhoyt Loading

Change Summary

Overview

New Dependencies

New Files

Testing

laspsandoval commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laspsandoval Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bourque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech3371 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laspsandoval commented Nov 15, 2023 •

edited by sdhoyt

Loading

laspsandoval Dec 12, 2023 •

edited

Loading