No longer keep fits files open unless we are actually reading them. Closes #7

Author: gsleap

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mwalib"
-version = "0.4.1"
+version = "0.4.2"
 homepage = "https://github.com/MWATelescope/mwalib"
 authors = ["Greg Sleap <greg.sleap@curtin.edu.au>",
            "Christopher H. Jordan <christopherjordan87@gmail.com>"]

benches/bench.rs

@@ -1,5 +1,4 @@
 use criterion::*;
-use fitsio::*;
 
 use mwalib::*;
 

src/context.rs

@@ -264,7 +264,7 @@ impl mwalibContext {
             (gt * 1000.).round() as _
         };
 
-        let (mut gpubox_info, timesteps) =
+        let (gpubox_info, timesteps) =
         // Do gpubox stuff only if we have gpubox files.
             if !gpuboxes.is_empty() {
                 let gpubox_info = examine_gpubox_files(&gpuboxes)?;
@@ -388,7 +388,7 @@ impl mwalibContext {
             let (batch_index, _) =
                 gpubox_info.time_map[&timesteps[0].unix_time_ms][&coarse_channel];
 
-            let mut fptr = &mut gpubox_info.batches[batch_index].gpubox_files[0].fptr;
+            let mut fptr = fits_open!(&gpubox_info.batches[batch_index].gpubox_files[0].filename)?;
 
             mwalibContext::validate_first_hdu(
                 gpubox_info.corr_format,
@@ -753,8 +753,9 @@ impl mwalibContext {
         if self.gpubox_batches.is_empty() {
             return Err(GpuboxError::NoGpuboxes);
         }
-        let mut fptr =
-            &mut self.gpubox_batches[batch_index].gpubox_files[coarse_channel_index].fptr;
+        let mut fptr = fits_open!(
+            &self.gpubox_batches[batch_index].gpubox_files[coarse_channel_index].filename
+        )?;
         let hdu = fits_open_hdu!(&mut fptr, hdu_index)?;
         output_buffer = get_fits_image!(&mut fptr, &hdu)?;
         // If legacy correlator, then convert the HDU into the correct output format
@@ -817,8 +818,9 @@ impl mwalibContext {
         if self.gpubox_batches.is_empty() {
             return Err(GpuboxError::NoGpuboxes);
         }
-        let mut fptr =
-            &mut self.gpubox_batches[batch_index].gpubox_files[coarse_channel_index].fptr;
+        let mut fptr = fits_open!(
+            &self.gpubox_batches[batch_index].gpubox_files[coarse_channel_index].filename
+        )?;
         let hdu = fits_open_hdu!(&mut fptr, hdu_index)?;
         output_buffer = get_fits_image!(&mut fptr, &hdu)?;
         // If legacy correlator, then convert the HDU into the correct output format
@@ -1372,14 +1374,15 @@ mod tests {
         //
         // Open a context and load in a test metafits and gpubox file
         let gpuboxfiles = vec![filename];
-        let mut context = mwalibContext::new(&metafits_filename, &gpuboxfiles)
+        let context = mwalibContext::new(&metafits_filename, &gpuboxfiles)
             .expect("Failed to create mwalibContext");
 
         let coarse_channel = context.coarse_channels[0].gpubox_number;
         let (batch_index, _) =
             context.gpubox_time_map[&context.timesteps[0].unix_time_ms][&coarse_channel];
 
-        let mut fptr = &mut context.gpubox_batches[batch_index].gpubox_files[0].fptr;
+        let mut fptr =
+            fits_open!(&context.gpubox_batches[batch_index].gpubox_files[0].filename).unwrap();
 
         let result_valid = mwalibContext::validate_first_hdu(
             context.corr_version,

src/fits_read/mod.rs

@@ -636,24 +636,24 @@ mod tests {
 
             // Verify that using the normal `fitsio` gives the wrong result, unless
             // the type is an i64.
-            let foo_i32 = hdu.read_key::<i32>(fptr, "foo");
-            let foo_i64 = hdu.read_key::<i64>(fptr, "foo");
-            assert!(foo_i32.is_ok());
-            assert!(foo_i64.is_ok());
-            assert_eq!(foo_i32.unwrap(), 1);
-            assert_eq!(foo_i64.unwrap(), 10);
-
-            // Despite writing to "foo", the key is written as "FOO".
-            let foo_i64 = hdu.read_key::<i64>(fptr, "FOO");
-            assert!(foo_i64.is_ok());
-            assert_eq!(foo_i64.unwrap(), 10);
-
-            let foo_u8: Result<u8, _> = get_required_fits_key!(fptr, &hdu, "foo");
-            let foo_i8: Result<i8, _> = get_required_fits_key!(fptr, &hdu, "foo");
-            assert!(foo_u8.is_ok());
-            assert!(foo_i8.is_ok());
-            assert_eq!(foo_u8.unwrap(), 10);
-            assert_eq!(foo_i8.unwrap(), 10);
+            let fits_value_i32 = hdu.read_key::<i32>(fptr, "FOO");
+            let fits_value_i64 = hdu.read_key::<i64>(fptr, "FOO");
+            assert!(fits_value_i32.is_ok());
+            assert!(fits_value_i64.is_ok());
+            assert_eq!(fits_value_i32.unwrap(), 1);
+            assert_eq!(fits_value_i64.unwrap(), 10);
+
+            // Despite writing to "fits_value", the key is written as "FOO".
+            let fits_value_i64 = hdu.read_key::<i64>(fptr, "FOO");
+            assert!(fits_value_i64.is_ok());
+            assert_eq!(fits_value_i64.unwrap(), 10);
+
+            let fits_value_u8: Result<u8, _> = get_required_fits_key!(fptr, &hdu, "foo");
+            let fits_value_i8: Result<i8, _> = get_required_fits_key!(fptr, &hdu, "foo");
+            assert!(fits_value_u8.is_ok());
+            assert!(fits_value_i8.is_ok());
+            assert_eq!(fits_value_u8.unwrap(), 10);
+            assert_eq!(fits_value_i8.unwrap(), 10);
 
             // Can't parse the negative number into a unsigned int.
             let bar_u8: Result<u8, _> = get_required_fits_key!(fptr, &hdu, "bar");
@@ -671,9 +671,9 @@ mod tests {
             let hdu = fptr.hdu(0).expect("Couldn't open HDU 0");
 
             // Failure to get a key that doesn't exist is OK if we're using the optional variant.
-            let foo: Result<Option<u8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
-            assert!(foo.is_ok());
-            assert!(foo.unwrap().is_none());
+            let fits_value: Result<Option<u8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
+            assert!(fits_value.is_ok());
+            assert!(fits_value.unwrap().is_none());
 
             // Key types must be i64 to get any sort of sanity.
             hdu.write_key(fptr, "foo", 10i64)
@@ -683,24 +683,24 @@ mod tests {
 
             // Verify that using the normal `fitsio` gives the wrong result, unless
             // the type is an i64.
-            let foo_i32 = hdu.read_key::<i32>(fptr, "foo");
-            let foo_i64 = hdu.read_key::<i64>(fptr, "foo");
-            assert!(foo_i32.is_ok());
-            assert!(foo_i64.is_ok());
-            assert_eq!(foo_i32.unwrap(), 1);
-            assert_eq!(foo_i64.unwrap(), 10);
+            let fits_value_i32 = hdu.read_key::<i32>(fptr, "FOO");
+            let fits_value_i64 = hdu.read_key::<i64>(fptr, "FOO");
+            assert!(fits_value_i32.is_ok());
+            assert!(fits_value_i64.is_ok());
+            assert_eq!(fits_value_i32.unwrap(), 1);
+            assert_eq!(fits_value_i64.unwrap(), 10);
 
             // Despite writing to "foo", the key is written as "FOO".
-            let foo_i64 = hdu.read_key::<i64>(fptr, "FOO");
-            assert!(foo_i64.is_ok());
-            assert_eq!(foo_i64.unwrap(), 10);
+            let fits_value_i64 = hdu.read_key::<i64>(fptr, "FOO");
+            assert!(fits_value_i64.is_ok());
+            assert_eq!(fits_value_i64.unwrap(), 10);
 
-            let foo_u8: Result<Option<u8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
-            let foo_i8: Result<Option<i8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
-            assert!(foo_u8.is_ok());
-            assert!(foo_i8.is_ok());
-            assert_eq!(foo_u8.unwrap(), Some(10));
-            assert_eq!(foo_i8.unwrap(), Some(10));
+            let fits_value_u8: Result<Option<u8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
+            let fits_value_i8: Result<Option<i8>, _> = get_optional_fits_key!(fptr, &hdu, "foo");
+            assert!(fits_value_u8.is_ok());
+            assert!(fits_value_i8.is_ok());
+            assert_eq!(fits_value_u8.unwrap(), Some(10));
+            assert_eq!(fits_value_i8.unwrap(), Some(10));
 
             // Can't parse the negative number into a unsigned int.
             let bar_u8: Result<u8, _> = get_required_fits_key!(fptr, &hdu, "bar");
@@ -719,18 +719,19 @@ mod tests {
 
             // Failure to get a key that doesn't exist.
             let does_not_exist: Result<String, FitsError> =
-                get_required_fits_key!(fptr, &hdu, "foo");
+                get_required_fits_key!(fptr, &hdu, "fits_value");
             assert!(does_not_exist.is_err());
 
             // Add a test string
-            hdu.write_key(fptr, "foo", "hello")
-                .expect("Couldn't write key 'foo'");
+            hdu.write_key(fptr, "fits_value", "hello")
+                .expect("Couldn't write key 'fits_value'");
 
-            // Read foo back in
-            let foo_string: String = get_required_fits_key!(fptr, &hdu, "foo").unwrap();
+            // Read fits_value back in
+            let fits_value_string: String =
+                get_required_fits_key!(fptr, &hdu, "fits_value").unwrap();
 
-            // Despite writing to "foo", the key is written as "FOO".
-            assert_eq!(foo_string, "hello");
+            // Despite writing to "fits_value", the key is written as "FOO".
+            assert_eq!(fits_value_string, "hello");
         });
     }
 
@@ -742,22 +743,22 @@ mod tests {
 
             // No Failure to get a key that doesn't exist.
             let does_not_exist: Result<Option<String>, FitsError> =
-                get_optional_fits_key!(fptr, &hdu, "foo");
+                get_optional_fits_key!(fptr, &hdu, "fits_value");
 
             assert!(does_not_exist.is_ok());
             assert!(does_not_exist.unwrap().is_none());
 
             // Add a test string
-            hdu.write_key(fptr, "foo", "hello")
-                .expect("Couldn't write key 'foo'");
+            hdu.write_key(fptr, "fits_value", "hello")
+                .expect("Couldn't write key 'fits_value'");
 
-            // Read foo back in
-            let foo_string: Result<Option<String>, FitsError> =
-                get_optional_fits_key!(fptr, &hdu, "foo");
+            // Read fits_value back in
+            let fits_value_string: Result<Option<String>, FitsError> =
+                get_optional_fits_key!(fptr, &hdu, "fits_value");
 
-            // Despite writing to "foo", the key is written as "FOO".
-            assert!(foo_string.is_ok());
-            assert_eq!(foo_string.unwrap().unwrap(), "hello");
+            // Despite writing to "fits_value", the key is written as "FOO".
+            assert!(fits_value_string.is_ok());
+            assert_eq!(fits_value_string.unwrap().unwrap(), "hello");
         });
     }
 
@@ -788,18 +789,18 @@ mod tests {
                 );
             }
 
-            let (status, foo_str) = unsafe { get_fits_long_string(fptr.as_raw(), "foo") };
+            let (status, fits_value_str) = unsafe { get_fits_long_string(fptr.as_raw(), "FOO") };
             assert_eq!(status, 0);
-            assert_eq!(foo_str, complete_string);
+            assert_eq!(fits_value_str, complete_string);
 
             // Try out the `fitsio` read key.
             let hdu = fptr.hdu(0).expect("Couldn't open HDU 0");
-            let fitsio_str = hdu.read_key::<String>(fptr, "foo");
+            let fitsio_str = hdu.read_key::<String>(fptr, "FOO");
             assert!(fitsio_str.is_ok());
             assert_eq!(fitsio_str.unwrap(), first_string);
 
             // A repeated read just returns the first string again.
-            let fitsio_str = hdu.read_key::<String>(fptr, "foo");
+            let fitsio_str = hdu.read_key::<String>(fptr, "FOO");
             assert!(fitsio_str.is_ok());
             assert_eq!(fitsio_str.unwrap(), first_string);
         });
@@ -815,8 +816,8 @@ mod tests {
             // with CONTINUE statements. We have to do it for ourselves.
             unsafe {
                 let fptr_ffi = fptr.as_raw();
-                let keyword_ffi = CString::new("foo")
-                    .expect("get_fits_long_string: CString::new() failed for 'foo'");
+                let keyword_ffi = CString::new("fits_value")
+                    .expect("get_fits_long_string: CString::new() failed for 'fits_value'");
                 let value_ffi = CString::new(complete_string)
                     .expect("get_fits_long_string: CString::new() failed for 'complete_string'");
                 let mut status = 0;
@@ -830,7 +831,7 @@ mod tests {
                 );
             }
 
-            let (status, _) = unsafe { get_fits_long_string(fptr.as_raw(), "NOTfoo") };
+            let (status, _) = unsafe { get_fits_long_string(fptr.as_raw(), "NOTfits_value") };
             assert_ne!(status, 0);
         });
     }

src/gpubox/mod.rs

@@ -59,8 +59,6 @@ pub struct GPUBoxFile {
     pub filename: String,
     /// channel number (Legacy==gpubox host number 01..24; V2==receiver channel number 001..255)
     pub channel_identifier: usize,
-    /// Pointer to fits file
-    pub fptr: FitsFile,
 }
 
 #[cfg(not(tarpaulin_include))]
@@ -147,11 +145,9 @@ fn convert_temp_gpuboxes(
     }
 
     for temp_g in temp_gpuboxes.into_iter() {
-        let fptr = fits_open!(&temp_g.filename)?;
         let g = GPUBoxFile {
             filename: temp_g.filename.to_string(),
             channel_identifier: temp_g.channel_identifier,
-            fptr,
         };
         gpubox_batches[temp_g.batch_number].gpubox_files.push(g);
     }
@@ -236,8 +232,10 @@ pub fn examine_gpubox_files<T: AsRef<Path>>(
     let mut hdu_size: Option<usize> = None;
     for b in &mut batches {
         for g in &mut b.gpubox_files {
-            let hdu = fits_open_hdu!(&mut g.fptr, 1)?;
-            let this_size = get_hdu_image_size!(&mut g.fptr, &hdu)?.iter().product();
+            let mut fptr = fits_open!(&g.filename)?;
+
+            let hdu = fits_open_hdu!(&mut fptr, 1)?;
+            let this_size = get_hdu_image_size!(&mut fptr, &hdu)?.iter().product();
             match hdu_size {
                 None => hdu_size = Some(this_size),
                 Some(s) => {