Added some coarse_channel tests. Fixed more clippy lints. Bumped to 0…

….6.1

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 Changes in each release are listed below.
 
+## 0.6.1 08-Mar-2021 (Pre-release)
+
+* Fixed za (zenith angle) calculation.
+* Added more comprehensive testing for some coarse_channel methods.
+* Addressed many clippy lints.
+
 ## 0.6.0 05-Mar-2021 (Pre-release)
 
 * Minor updates to enable packaging and deployment to crates.io.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mwalib"
-version = "0.6.0"
+version = "0.6.1"
 homepage = "https://github.com/MWATelescope/mwalib"
 repository = "https://github.com/MWATelescope/mwalib"
 readme = "README.md"

src/coarse_channel/test.rs

@@ -259,6 +259,15 @@ fn test_process_coarse_chans_no_time_maps_legacy() {
     let coarse_chan_array = result.unwrap();
 
     assert_eq!(coarse_chan_array.len(), 3);
+    assert_eq!(coarse_chan_array[0].rec_chan_number, 133);
+    assert_eq!(coarse_chan_array[0].corr_chan_number, 2);
+    assert_eq!(coarse_chan_array[0].gpubox_number, 3);
+    assert_eq!(coarse_chan_array[1].rec_chan_number, 134);
+    assert_eq!(coarse_chan_array[1].corr_chan_number, 1);
+    assert_eq!(coarse_chan_array[1].gpubox_number, 2);
+    assert_eq!(coarse_chan_array[2].rec_chan_number, 135);
+    assert_eq!(coarse_chan_array[2].corr_chan_number, 0);
+    assert_eq!(coarse_chan_array[2].gpubox_number, 1);
 }
 
 #[test]
@@ -280,6 +289,15 @@ fn test_process_coarse_chans_no_time_maps_mwax_v2() {
     let coarse_chan_array = result.unwrap();
 
     assert_eq!(coarse_chan_array.len(), 3);
+    assert_eq!(coarse_chan_array[0].rec_chan_number, 133);
+    assert_eq!(coarse_chan_array[0].corr_chan_number, 0);
+    assert_eq!(coarse_chan_array[0].gpubox_number, 133);
+    assert_eq!(coarse_chan_array[1].rec_chan_number, 134);
+    assert_eq!(coarse_chan_array[1].corr_chan_number, 1);
+    assert_eq!(coarse_chan_array[1].gpubox_number, 134);
+    assert_eq!(coarse_chan_array[2].rec_chan_number, 135);
+    assert_eq!(coarse_chan_array[2].corr_chan_number, 2);
+    assert_eq!(coarse_chan_array[2].gpubox_number, 135);
 }
 
 #[test]

src/correlator_context/mod.rs

@@ -61,7 +61,7 @@ pub struct CorrelatorContext {
     /// corresponds directly to other gpubox-related objects
     /// (e.g. `gpubox_hdu_limits`). Structured:
     /// `gpubox_batches[batch][filename]`.
-    pub(crate) gpubox_batches: Vec<GPUBoxBatch>,
+    pub(crate) gpubox_batches: Vec<GpuBoxBatch>,
     /// We assume as little as possible about the data layout in the gpubox
     /// files; here, a `BTreeMap` contains each unique UNIX time from every
     /// gpubox, which is associated with another `BTreeMap`, associating each

src/correlator_context/test.rs

@@ -338,14 +338,15 @@ fn test_validate_hdu_axes_naxis_mismatches_oldlegacy() {
     let metafits_fine_chans_per_coarse = 128;
     let metafits_baselines = 8256;
     let visibility_pols = 4;
+    let values = 1;
 
     // Check for NAXIS1 mismatch
     let result_bad1 = CorrelatorContext::validate_hdu_axes(
         CorrelatorVersion::OldLegacy,
         metafits_fine_chans_per_coarse,
         metafits_baselines,
         visibility_pols,
-        8256 * 4 * 1,
+        metafits_baselines * visibility_pols * values,
         128,
     );
 
@@ -385,14 +386,15 @@ fn test_validate_hdu_axes_naxis_mismatches_legacy() {
     let metafits_fine_chans_per_coarse = 128;
     let metafits_baselines = 8256;
     let visibility_pols = 4;
+    let values = 1;
 
     // Check for NAXIS1 mismatch
     let result_bad1 = CorrelatorContext::validate_hdu_axes(
         CorrelatorVersion::Legacy,
         metafits_fine_chans_per_coarse,
         metafits_baselines,
         visibility_pols,
-        8256 * 4 * 1,
+        metafits_baselines * visibility_pols * values,
         128,
     );
 
@@ -432,14 +434,15 @@ fn test_validate_hdu_axes_naxis_mismatches_v2() {
     let metafits_fine_chans_per_coarse = 128;
     let metafits_baselines = 8256;
     let visibility_pols = 4;
+    let values = 2;
 
     // Check for NAXIS1 mismatch
     let result_bad1 = CorrelatorContext::validate_hdu_axes(
         CorrelatorVersion::V2,
         metafits_fine_chans_per_coarse,
         metafits_baselines,
         visibility_pols,
-        128 * 4 * 1,
+        metafits_fine_chans_per_coarse * visibility_pols,
         8256,
     );
 
@@ -460,7 +463,7 @@ fn test_validate_hdu_axes_naxis_mismatches_v2() {
         metafits_fine_chans_per_coarse,
         metafits_baselines,
         visibility_pols,
-        128 * 4 * 2,
+        metafits_fine_chans_per_coarse * visibility_pols * values,
         8257,
     );
 

src/ffi/mod.rs

@@ -1847,7 +1847,6 @@ pub unsafe extern "C" fn mwalib_coarse_channels_free(
 
 /// Representation in C of an `RFInput` struct
 #[repr(C)]
-#[allow(clippy::upper_case_acronyms)]
 pub struct RFInput {
     /// This is the metafits order (0-n inputs)
     pub input: u32,

src/fits_read/test.rs

@@ -12,6 +12,7 @@ use crate::*;
 use fitsio::images::{ImageDescription, ImageType};
 use fitsio::tables::{ColumnDataType, ColumnDescription};
 use fitsio_sys::ffpkls;
+use float_cmp::*;
 
 #[test]
 fn test_get_hdu_image_size_image() {
@@ -296,7 +297,7 @@ fn test_1101503312_metafits() -> Result<(), FitsError> {
     let mut fptr = fits_open!(&metafits)?;
     let hdu = fits_open_hdu!(&mut fptr, 0)?;
     let freq_centre: f64 = get_required_fits_key!(&mut fptr, &hdu, "FREQCENT")?;
-    assert_eq!(freq_centre, 154.24);
+    assert!(approx_eq!(f64, freq_centre, 154.24, F64Margin::default()));
     let fine_chan_width: u8 = get_required_fits_key!(&mut fptr, &hdu, "FINECHAN")?;
     assert_eq!(fine_chan_width, 10);
 
@@ -311,7 +312,7 @@ fn test_1101503312_metafits() -> Result<(), FitsError> {
 
     let hdu = fits_open_hdu!(&mut fptr, 1)?;
     let east: Vec<f32> = get_fits_col!(&mut fptr, &hdu, "East")?;
-    assert_eq!(east[0], -585.675);
+    assert!(approx_eq!(f32, east[0], -585.675, F32Margin::default()));
     let doesnt_exist: Result<Vec<f32>, FitsError> = get_fits_col!(&mut fptr, &hdu, "South");
     assert!(doesnt_exist.is_err());
     Ok(())
@@ -323,7 +324,7 @@ fn test_1244973688_metafits() -> Result<(), FitsError> {
     let mut fptr = fits_open!(&metafits)?;
     let hdu = fits_open_hdu!(&mut fptr, 0)?;
     let freq_centre: f64 = get_required_fits_key!(&mut fptr, &hdu, "FREQCENT")?;
-    assert_eq!(freq_centre, 147.84);
+    assert!(approx_eq!(f64, freq_centre, 147.84, F64Margin::default()));
     let fine_chan_width: u8 = get_required_fits_key!(&mut fptr, &hdu, "FINECHAN")?;
     assert_eq!(fine_chan_width, 10);
 
@@ -338,7 +339,7 @@ fn test_1244973688_metafits() -> Result<(), FitsError> {
 
     let hdu = fits_open_hdu!(&mut fptr, 1)?;
     let east: Vec<f32> = get_fits_col!(&mut fptr, &hdu, "East")?;
-    assert_eq!(east[0], -585.675);
+    assert!(approx_eq!(f32, east[0], -585.675, F32Margin::default()));
     let doesnt_exist: Result<Vec<f32>, FitsError> = get_fits_col!(&mut fptr, &hdu, "South");
     assert!(doesnt_exist.is_err());
     Ok(())

src/gpubox_files/error.rs

@@ -9,7 +9,6 @@ Errors associated with reading in gpubox files.
 use thiserror::Error;
 
 #[derive(Error, Debug)]
-#[allow(clippy::upper_case_acronyms)]
 pub enum GpuboxError {
     #[error("Invalid timestep index provided. The timestep index must be between 0 and {0}")]
     InvalidTimeStepIndex(usize),

src/gpubox_files/mod.rs

@@ -30,13 +30,12 @@ pub(crate) struct ObsTimes {
 
 /// This represents one group of gpubox files with the same "batch" identitifer.
 /// e.g. obsid_datetime_chan_batch
-#[allow(clippy::upper_case_acronyms)]
-pub(crate) struct GPUBoxBatch {
+pub(crate) struct GpuBoxBatch {
     pub batch_number: usize,           // 00,01,02..n
-    pub gpubox_files: Vec<GPUBoxFile>, // Vector storing the details of each gpubox file in this batch
+    pub gpubox_files: Vec<GpuBoxFile>, // Vector storing the details of each gpubox file in this batch
 }
 
-impl GPUBoxBatch {
+impl GpuBoxBatch {
     pub fn new(batch_number: usize) -> Self {
         Self {
             batch_number,
@@ -45,7 +44,7 @@ impl GPUBoxBatch {
     }
 }
 
-impl fmt::Debug for GPUBoxBatch {
+impl fmt::Debug for GpuBoxBatch {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
@@ -56,15 +55,14 @@ impl fmt::Debug for GPUBoxBatch {
 }
 
 /// This represents one gpubox file
-#[allow(clippy::upper_case_acronyms)]
-pub(crate) struct GPUBoxFile {
+pub(crate) struct GpuBoxFile {
     /// Filename of gpubox file
     pub filename: String,
     /// channel number (Legacy==gpubox host number 01..24; V2==receiver channel number 001..255)
     pub channel_identifier: usize,
 }
 
-impl fmt::Debug for GPUBoxFile {
+impl fmt::Debug for GpuBoxFile {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
@@ -74,22 +72,21 @@ impl fmt::Debug for GPUBoxFile {
     }
 }
 
-impl std::cmp::PartialEq for GPUBoxBatch {
+impl std::cmp::PartialEq for GpuBoxBatch {
     fn eq(&self, other: &Self) -> bool {
         self.batch_number == other.batch_number && self.gpubox_files == other.gpubox_files
     }
 }
 
-impl std::cmp::PartialEq for GPUBoxFile {
+impl std::cmp::PartialEq for GpuBoxFile {
     fn eq(&self, other: &Self) -> bool {
         self.filename == other.filename && self.channel_identifier == other.channel_identifier
     }
 }
 
 /// A temporary representation of a gpubox file
 #[derive(Clone, Debug)]
-#[allow(clippy::upper_case_acronyms)]
-struct TempGPUBoxFile<'a> {
+struct TempGpuBoxFile<'a> {
     /// Filename of gpubox file
     filename: &'a str,
     /// Channel number (Legacy==gpubox host number 01..24; V2==receiver channel number 001..255)
@@ -98,7 +95,7 @@ struct TempGPUBoxFile<'a> {
     batch_number: usize,
 }
 
-impl<'a> std::cmp::PartialEq for TempGPUBoxFile<'a> {
+impl<'a> std::cmp::PartialEq for TempGpuBoxFile<'a> {
     fn eq(&self, other: &Self) -> bool {
         self.filename == other.filename
             && self.channel_identifier == other.channel_identifier
@@ -135,7 +132,7 @@ pub(crate) type GpuboxTimeMap = BTreeMap<u64, BTreeMap<usize, (usize, usize)>>;
 /// A little struct to help us not get confused when dealing with the returned
 /// values from complex functions.
 pub(crate) struct GpuboxInfo {
-    pub batches: Vec<GPUBoxBatch>,
+    pub batches: Vec<GpuBoxBatch>,
     pub corr_format: CorrelatorVersion,
     pub time_map: GpuboxTimeMap,
     pub hdu_size: usize,
@@ -161,17 +158,17 @@ pub(crate) struct GpuboxInfo {
 /// * A Result containing a vector of `GPUBoxBatch`.
 ///
 ///
-fn convert_temp_gpuboxes(temp_gpuboxes: Vec<TempGPUBoxFile>) -> Vec<GPUBoxBatch> {
+fn convert_temp_gpuboxes(temp_gpuboxes: Vec<TempGpuBoxFile>) -> Vec<GpuBoxBatch> {
     // unwrap is safe as a check is performed above to ensure that there are
     // some files present.
     let num_batches = temp_gpuboxes.iter().map(|g| g.batch_number).max().unwrap() + 1;
-    let mut gpubox_batches: Vec<GPUBoxBatch> = Vec::with_capacity(num_batches);
+    let mut gpubox_batches: Vec<GpuBoxBatch> = Vec::with_capacity(num_batches);
     for b in 0..num_batches {
-        gpubox_batches.push(GPUBoxBatch::new(b));
+        gpubox_batches.push(GpuBoxBatch::new(b));
     }
 
     for temp_g in temp_gpuboxes.into_iter() {
-        let g = GPUBoxFile {
+        let g = GpuBoxFile {
             filename: temp_g.filename.to_string(),
             channel_identifier: temp_g.channel_identifier,
         };
@@ -309,12 +306,12 @@ pub(crate) fn examine_gpubox_files<T: AsRef<Path>>(
 ///
 fn determine_gpubox_batches<T: AsRef<Path>>(
     gpubox_filenames: &[T],
-) -> Result<(Vec<TempGPUBoxFile>, CorrelatorVersion, usize), GpuboxError> {
+) -> Result<(Vec<TempGpuBoxFile>, CorrelatorVersion, usize), GpuboxError> {
     if gpubox_filenames.is_empty() {
         return Err(GpuboxError::NoGpuboxes);
     }
     let mut format = None;
-    let mut temp_gpuboxes: Vec<TempGPUBoxFile> = Vec::with_capacity(gpubox_filenames.len());
+    let mut temp_gpuboxes: Vec<TempGpuBoxFile> = Vec::with_capacity(gpubox_filenames.len());
 
     for g_path in gpubox_filenames {
         // So that we can pass along useful error messages, convert the input
@@ -338,7 +335,7 @@ fn determine_gpubox_batches<T: AsRef<Path>>(
 
                 // The following unwraps are safe, because the regex wouldn't
                 // work if they couldn't be parsed into ints.
-                temp_gpuboxes.push(TempGPUBoxFile {
+                temp_gpuboxes.push(TempGpuBoxFile {
                     filename: g,
                     channel_identifier: caps["channel"].parse().unwrap(),
                     batch_number: caps["batch"].parse().unwrap(),
@@ -354,7 +351,7 @@ fn determine_gpubox_batches<T: AsRef<Path>>(
                         _ => return Err(GpuboxError::Mixture),
                     }
 
-                    temp_gpuboxes.push(TempGPUBoxFile {
+                    temp_gpuboxes.push(TempGpuBoxFile {
                         filename: g,
                         channel_identifier: caps["band"].parse().unwrap(),
                         batch_number: caps["batch"].parse().unwrap(),
@@ -370,7 +367,7 @@ fn determine_gpubox_batches<T: AsRef<Path>>(
                             _ => return Err(GpuboxError::Mixture),
                         }
 
-                        temp_gpuboxes.push(TempGPUBoxFile {
+                        temp_gpuboxes.push(TempGpuBoxFile {
                             filename: g,
                             channel_identifier: caps["band"].parse().unwrap(),
                             // There's only one batch.
@@ -596,7 +593,7 @@ fn validate_gpubox_metadata_obs_id(
 ///
 ///
 fn create_time_map(
-    gpuboxes: &[TempGPUBoxFile],
+    gpuboxes: &[TempGpuBoxFile],
     correlator_version: CorrelatorVersion,
 ) -> Result<GpuboxTimeMap, GpuboxError> {
     // Ugly hack to open up all the HDUs of the gpubox files in parallel. We