improvements from external prs

-fix corner case where dr converter failed when initializing

Closes twitter#550

Author: twitter-team

navi/dr_transform/src/all_config.rs

@@ -44,5 +44,6 @@ pub struct RenamedFeatures {
 }
 
 pub fn parse(json_str: &str) -> Result<AllConfig, Error> {
-    serde_json::from_str(json_str)
+    let all_config: AllConfig = serde_json::from_str(json_str)?;
+    Ok(all_config)
 }

navi/dr_transform/src/converter.rs

@@ -122,7 +122,7 @@ enum FullTypeId {
   //   TFT_TENSOR[TFT_INT32, TFT_UNKNOWN]
   //     is a Tensor of int32 element type and unknown shape.
   //
-  // TODO: Define TFT_SHAPE and add more examples.
+  // TODO(mdan): Define TFT_SHAPE and add more examples.
   TFT_TENSOR = 1000;
 
   // Array (or tensorflow::TensorList in the variant type registry).
@@ -178,7 +178,7 @@ enum FullTypeId {
   // object (for now).
 
   // The bool element type.
-  // TODO
+  // TODO(mdan): Quantized types, legacy representations (e.g. ref)
   TFT_BOOL = 200;
   // Integer element types.
   TFT_UINT8 = 201;
@@ -195,7 +195,7 @@ enum FullTypeId {
   TFT_DOUBLE = 211;
   TFT_BFLOAT16 = 215;
   // Complex element types.
-  // TODO: Represent as TFT_COMPLEX[TFT_DOUBLE] instead?
+  // TODO(mdan): Represent as TFT_COMPLEX[TFT_DOUBLE] instead?
   TFT_COMPLEX64 = 212;
   TFT_COMPLEX128 = 213;
   // The string element type.
@@ -240,7 +240,7 @@ enum FullTypeId {
   // ownership is in the true sense: "the op argument representing the lock is
   // available".
   // Mutex locks are the dynamic counterpart of control dependencies.
-  // TODO: Properly document this thing.
+  // TODO(mdan): Properly document this thing.
   //
   // Parametrization: TFT_MUTEX_LOCK[].
   TFT_MUTEX_LOCK = 10202;
@@ -271,6 +271,6 @@ message FullTypeDef {
   oneof attr {
     string s = 3;
     int64 i = 4;
-    // TODO: list/tensor, map? Need to reconcile with TFT_RECORD, etc.
+    // TODO(mdan): list/tensor, map? Need to reconcile with TFT_RECORD, etc.
   }
 }

navi/navi/proto/tensorflow/core/framework/full_type.proto

@@ -23,7 +23,7 @@ message FunctionDefLibrary {
 // with a value. When a GraphDef has a call to a function, it must
 // have binding for every attr defined in the signature.
 //
-// TODO:
+// TODO(zhifengc):
 //   * device spec, etc.
 message FunctionDef {
   // The definition of the function's name, arguments, return values,

navi/navi/proto/tensorflow/core/framework/function.proto

@@ -61,7 +61,7 @@ message NodeDef {
   // one of the names from the corresponding OpDef's attr field).
   // The values must have a type matching the corresponding OpDef
   // attr's type field.
-  // TODO: Add some examples here showing best practices.
+  // TODO(josh11b): Add some examples here showing best practices.
   map<string, AttrValue> attr = 5;
 
   message ExperimentalDebugInfo {

navi/navi/proto/tensorflow/core/framework/node_def.proto

@@ -96,7 +96,7 @@ message OpDef {
     // Human-readable description.
     string description = 4;
 
-    // TODO: bool is_optional?
+    // TODO(josh11b): bool is_optional?
 
     // --- Constraints ---
     // These constraints are only in effect if specified.  Default is no
@@ -139,7 +139,7 @@ message OpDef {
   // taking input from multiple devices with a tree of aggregate ops
   // that aggregate locally within each device (and possibly within
   // groups of nearby devices) before communicating.
-  // TODO: Implement that optimization.
+  // TODO(josh11b): Implement that optimization.
   bool is_aggregate = 16;  // for things like add
 
   // Other optimizations go here, like

navi/navi/proto/tensorflow/core/framework/op_def.proto

@@ -53,7 +53,7 @@ message MemoryStats {
 
 // Time/size stats recorded for a single execution of a graph node.
 message NodeExecStats {
-  // TODO: Use some more compact form of node identity than
+  // TODO(tucker): Use some more compact form of node identity than
   // the full string name.  Either all processes should agree on a
   // global id (cost_id?) for each node, or we should use a hash of
   // the name.

navi/navi/proto/tensorflow/core/framework/step_stats.proto

@@ -16,7 +16,7 @@ option go_package = "github.com/tensorflow/tensorflow/tensorflow/go/core/framewo
 message TensorProto {
   DataType dtype = 1;
 
-  // Shape of the tensor.  TODO: sort out the 0-rank issues.
+  // Shape of the tensor.  TODO(touts): sort out the 0-rank issues.
   TensorShapeProto tensor_shape = 2;
 
   // Only one of the representations below is set, one of "tensor_contents" and

navi/navi/proto/tensorflow/core/framework/tensor.proto

@@ -532,7 +532,7 @@ message ConfigProto {
 
     // We removed the flag client_handles_error_formatting. Marking the tag
     // number as reserved.
-    // TODO: Should we just remove this tag so that it can be
+    // TODO(shikharagarwal): Should we just remove this tag so that it can be
     // used in future for other purpose?
     reserved 2;
 
@@ -576,7 +576,7 @@ message ConfigProto {
     // - If isolate_session_state is true, session states are isolated.
     // - If isolate_session_state is false, session states are shared.
     //
-    // TODO: Add a single API that consistently treats
+    // TODO(b/129330037): Add a single API that consistently treats
     // isolate_session_state and ClusterSpec propagation.
     bool share_session_state_in_clusterspec_propagation = 8;
 
@@ -704,7 +704,7 @@ message ConfigProto {
 
 // Options for a single Run() call.
 message RunOptions {
-  // TODO Turn this into a TraceOptions proto which allows
+  // TODO(pbar) Turn this into a TraceOptions proto which allows
   // tracing to be controlled in a more orthogonal manner?
   enum TraceLevel {
     NO_TRACE = 0;
@@ -781,7 +781,7 @@ message RunMetadata {
   repeated GraphDef partition_graphs = 3;
 
   message FunctionGraphs {
-    // TODO: Include some sort of function/cache-key identifier?
+    // TODO(nareshmodi): Include some sort of function/cache-key identifier?
     repeated GraphDef partition_graphs = 1;
 
     GraphDef pre_optimization_graph = 2;

navi/navi/proto/tensorflow/core/protobuf/config.proto

@@ -194,7 +194,7 @@ service CoordinationService {
 
   // Report error to the task. RPC sets the receiving instance of coordination
   // service agent to error state permanently.
-  // TODO: Consider splitting this into a different RPC service.
+  // TODO(b/195990880): Consider splitting this into a different RPC service.
   rpc ReportErrorToAgent(ReportErrorToAgentRequest)
       returns (ReportErrorToAgentResponse);
 

navi/navi/proto/tensorflow/core/protobuf/coordination_service.proto

@@ -46,7 +46,7 @@ message DebugTensorWatch {
   // are to be debugged, the callers of Session::Run() must use distinct
   // debug_urls to make sure that the streamed or dumped events do not overlap
   // among the invocations.
-  // TODO: More visible documentation of this in g3docs.
+  // TODO(cais): More visible documentation of this in g3docs.
   repeated string debug_urls = 4;
 
   // Do not error out if debug op creation fails (e.g., due to dtype

navi/navi/proto/tensorflow/core/protobuf/debug.proto

@@ -12,7 +12,7 @@ option java_package = "org.tensorflow.util";
 option go_package = "github.com/tensorflow/tensorflow/tensorflow/go/core/protobuf/for_core_protos_go_proto";
 
 // Available modes for extracting debugging information from a Tensor.
-// TODO: Document the detailed column names and semantics in a separate
+// TODO(cais): Document the detailed column names and semantics in a separate
 // markdown file once the implementation settles.
 enum TensorDebugMode {
   UNSPECIFIED = 0;
@@ -223,7 +223,7 @@ message DebuggedDevice {
   // A debugger-generated ID for the device. Guaranteed to be unique within
   // the scope of the debugged TensorFlow program, including single-host and
   // multi-host settings.
-  // TODO: Test the uniqueness guarantee in multi-host settings.
+  // TODO(cais): Test the uniqueness guarantee in multi-host settings.
   int32 device_id = 2;
 }
 
@@ -264,7 +264,7 @@ message Execution {
   // field with the DebuggedDevice messages.
   repeated int32 output_tensor_device_ids = 9;
 
-  // TODO support, add more fields
+  // TODO(cais): When backporting to V1 Session.run() support, add more fields
   // such as fetches and feeds.
 }
 

navi/navi/proto/tensorflow/core/protobuf/debug_event.proto

@@ -7,7 +7,7 @@ option go_package = "github.com/tensorflow/tensorflow/tensorflow/go/core/protobu
 
 // Used to serialize and transmit tensorflow::Status payloads through
 // grpc::Status `error_details` since grpc::Status lacks payload API.
-// TODO: Use GRPC API once supported.
+// TODO(b/204231601): Use GRPC API once supported.
 message GrpcPayloadContainer {
   map<string, bytes> payloads = 1;
 }

navi/navi/proto/tensorflow/core/protobuf/distributed_runtime_payloads.proto

@@ -172,7 +172,7 @@ message WaitQueueDoneRequest {
 }
 
 message WaitQueueDoneResponse {
-  // TODO: Consider adding NodeExecStats here to be able to
+  // TODO(nareshmodi): Consider adding NodeExecStats here to be able to
   // propagate some stats.
 }
 

navi/navi/proto/tensorflow/core/protobuf/eager_service.proto

@@ -94,7 +94,7 @@ message ExtendSessionRequest {
 }
 
 message ExtendSessionResponse {
-  // TODO: Return something about the operation?
+  // TODO(mrry): Return something about the operation?
 
   // The new version number for the extended graph, to be used in the next call
   // to ExtendSession.

navi/navi/proto/tensorflow/core/protobuf/master.proto

@@ -176,7 +176,7 @@ message SavedBareConcreteFunction {
   // allows the ConcreteFunction to be called with nest structure inputs. This
   // field may not be populated. If this field is absent, the concrete function
   // can only be called with flat inputs.
-  // TODO: support calling saved ConcreteFunction with structured
+  // TODO(b/169361281): support calling saved ConcreteFunction with structured
   // inputs in C++ SavedModel API.
   FunctionSpec function_spec = 4;
 }

navi/navi/proto/tensorflow/core/protobuf/saved_object_graph.proto

@@ -17,7 +17,7 @@ option go_package = "github.com/tensorflow/tensorflow/tensorflow/go/core/protobu
 
 // Special header that is associated with a bundle.
 //
-// TODO: maybe in the future, we can add information about
+// TODO(zongheng,zhifengc): maybe in the future, we can add information about
 // which binary produced this checkpoint, timestamp, etc. Sometime, these can be
 // valuable debugging information. And if needed, these can be used as defensive
 // information ensuring reader (binary version) of the checkpoint and the writer

navi/navi/proto/tensorflow/core/protobuf/tensor_bundle.proto

@@ -188,7 +188,7 @@ message DeregisterGraphRequest {
 }
 
 message DeregisterGraphResponse {
-  // TODO: Optionally add summary stats for the graph.
+  // TODO(mrry): Optionally add summary stats for the graph.
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -294,7 +294,7 @@ message RunGraphResponse {
 
   // If the request asked for execution stats, the cost graph, or the partition
   // graphs, these are returned here.
-  // TODO: Package these in a RunMetadata instead.
+  // TODO(suharshs): Package these in a RunMetadata instead.
   StepStats step_stats = 2;
   CostGraphDef cost_graph = 3;
   repeated GraphDef partition_graph = 4;

navi/navi/proto/tensorflow/core/protobuf/worker.proto

@@ -13,5 +13,5 @@ message LogMetadata {
   SamplingConfig sampling_config = 2;
   // List of tags used to load the relevant MetaGraphDef from SavedModel.
   repeated string saved_model_tags = 3;
-  // TODO: Add more metadata as mentioned in the bug.
+  // TODO(b/33279154): Add more metadata as mentioned in the bug.
 }

navi/navi/proto/tensorflow_serving/apis/logging.proto

@@ -58,7 +58,7 @@ message FileSystemStoragePathSourceConfig {
 
   // A single servable name/base_path pair to monitor.
   // DEPRECATED: Use 'servables' instead.
-  // TODO: Stop using these fields, and ultimately remove them here.
+  // TODO(b/30898016): Stop using these fields, and ultimately remove them here.
   string servable_name = 1 [deprecated = true];
   string base_path = 2 [deprecated = true];
 
@@ -76,7 +76,7 @@ message FileSystemStoragePathSourceConfig {
   // check for a version to appear later.)
   // DEPRECATED: Use 'servable_versions_always_present' instead, which includes
   // this behavior.
-  // TODO: Remove 2019-10-31 or later.
+  // TODO(b/30898016): Remove 2019-10-31 or later.
   bool fail_if_zero_versions_at_startup = 4 [deprecated = true];
 
   // If true, the servable is always expected to exist on the underlying

navi/navi/proto/tensorflow_serving/config/file_system_storage_path_source.proto

@@ -9,7 +9,7 @@ import "tensorflow_serving/config/logging_config.proto";
 option cc_enable_arenas = true;
 
 // The type of model.
-// TODO: DEPRECATED.
+// TODO(b/31336131): DEPRECATED.
 enum ModelType {
   MODEL_TYPE_UNSPECIFIED = 0 [deprecated = true];
   TENSORFLOW = 1 [deprecated = true];
@@ -31,7 +31,7 @@ message ModelConfig {
   string base_path = 2;
 
   // Type of model.
-  // TODO: DEPRECATED. Please use 'model_platform' instead.
+  // TODO(b/31336131): DEPRECATED. Please use 'model_platform' instead.
   ModelType model_type = 3 [deprecated = true];
 
   // Type of model (e.g. "tensorflow").

navi/navi/proto/tensorflow_serving/config/model_server_config.proto

@@ -1,5 +1,6 @@
 use anyhow::Result;
 use log::{info, warn};
+use x509_parser::{prelude::{parse_x509_pem}, parse_x509_certificate};
 use std::collections::HashMap;
 use tokio::time::Instant;
 use tonic::{
@@ -27,6 +28,7 @@ use crate::cli_args::{ARGS, INPUTS, OUTPUTS};
 use crate::metrics::{
     NAVI_VERSION, NUM_PREDICTIONS, NUM_REQUESTS_FAILED, NUM_REQUESTS_FAILED_BY_MODEL,
     NUM_REQUESTS_RECEIVED, NUM_REQUESTS_RECEIVED_BY_MODEL, RESPONSE_TIME_COLLECTOR,
+    CERT_EXPIRY_EPOCH
 };
 use crate::predict_service::{Model, PredictService};
 use crate::tf_proto::tensorflow_serving::model_spec::VersionChoice::Version;
@@ -233,6 +235,12 @@ impl<T: Model> PredictionService for PredictService<T> {
     }
 }
 
+// A function that takes a timestamp as input and returns a ticker stream
+fn report_expiry(expiry_time: i64) {
+    info!("Certificate expires at epoch: {:?}", expiry_time);
+    CERT_EXPIRY_EPOCH.set(expiry_time as i64);
+}
+
 pub fn bootstrap<T: Model>(model_factory: ModelFactory<T>) -> Result<()> {
     info!("package: {}, version: {}, args: {:?}", NAME, VERSION, *ARGS);
     //we follow SemVer. So here we assume MAJOR.MINOR.PATCH
@@ -249,6 +257,7 @@ pub fn bootstrap<T: Model>(model_factory: ModelFactory<T>) -> Result<()> {
         );
     }
 
+    
     tokio::runtime::Builder::new_multi_thread()
         .thread_name("async worker")
         .worker_threads(ARGS.num_worker_threads)
@@ -266,6 +275,21 @@ pub fn bootstrap<T: Model>(model_factory: ModelFactory<T>) -> Result<()> {
             let mut builder = if ARGS.ssl_dir.is_empty() {
                 Server::builder()
             } else {
+                // Read the pem file as a string
+                let pem_str = std::fs::read_to_string(format!("{}/server.crt", ARGS.ssl_dir)).unwrap();
+                let res = parse_x509_pem(&pem_str.as_bytes());
+                match res {
+                    Ok((rem, pem_2)) => {
+                        assert!(rem.is_empty());
+                        assert_eq!(pem_2.label, String::from("CERTIFICATE"));
+                        let res_x509 = parse_x509_certificate(&pem_2.contents);
+                        info!("Certificate label: {}", pem_2.label);
+                        assert!(res_x509.is_ok());
+                        report_expiry(res_x509.unwrap().1.validity().not_after.timestamp());
+                    },
+                    _ => panic!("PEM parsing failed: {:?}", res),
+                }
+
                 let key = tokio::fs::read(format!("{}/server.key", ARGS.ssl_dir))
                     .await
                     .expect("can't find key file");
@@ -281,7 +305,7 @@ pub fn bootstrap<T: Model>(model_factory: ModelFactory<T>) -> Result<()> {
                 let identity = Identity::from_pem(pem.clone(), key);
                 let client_ca_cert = Certificate::from_pem(pem.clone());
                 let tls = ServerTlsConfig::new()
-                    .identity(identity)
+                    .identity(identity) 
                     .client_ca_root(client_ca_cert);
                 Server::builder()
                     .tls_config(tls)

navi/navi/src/bootstrap.rs

@@ -171,6 +171,9 @@ lazy_static! {
         &["model_name"]
     )
     .expect("metric can be created");
+    pub static ref CERT_EXPIRY_EPOCH: IntGauge =
+        IntGauge::new(":navi:cert_expiry_epoch", "Timestamp when the current cert expires")
+            .expect("metric can be created");
 }
 
 pub fn register_custom_metrics() {
@@ -249,6 +252,10 @@ pub fn register_custom_metrics() {
     REGISTRY
         .register(Box::new(CONVERTER_TIME_COLLECTOR.clone()))
         .expect("collector can be registered");
+    REGISTRY
+    .register(Box::new(CERT_EXPIRY_EPOCH.clone()))
+    .expect("collector can be registered");
+
 }
 
 pub fn register_dynamic_metrics(c: &HistogramVec) {

navi/navi/src/metrics.rs

@@ -189,7 +189,7 @@ pub mod onnx {
                     &version,
                     reporting_feature_ids,
                     Some(metrics::register_dynamic_metrics),
-                )),
+                )?),
             };
             onnx_model.warmup()?;
             Ok(onnx_model)

navi/navi/src/onnx_model.rs

@@ -24,7 +24,7 @@ use serde_json::{self, Value};
 
 pub trait Model: Send + Sync + Display + Debug + 'static {
     fn warmup(&self) -> Result<()>;
-    //TODO: refactor this to return Vec<Vec<TensorScores>>, i.e.
+    //TODO: refactor this to return vec<vec<TensorScores>>, i.e.
     //we have the underlying runtime impl to split the response to each client.
     //It will eliminate some inefficient memory copy in onnx_model.rs as well as simplify code
     fn do_predict(
@@ -222,8 +222,8 @@ impl<T: Model> PredictService<T> {
             .map(|b| b.parse().unwrap())
             .collect::<Vec<u64>>();
         let no_msg_wait_millis = *batch_time_out_millis.iter().min().unwrap();
-        let mut all_model_predictors =
-            ArrayVec::<ArrayVec<BatchPredictor<T>, MAX_VERSIONS_PER_MODEL>, MAX_NUM_MODELS>::new();
+        let mut all_model_predictors: ArrayVec::<ArrayVec<BatchPredictor<T>, MAX_VERSIONS_PER_MODEL>, MAX_NUM_MODELS> =
+            (0 ..MAX_NUM_MODELS).map( |_| ArrayVec::<BatchPredictor<T>, MAX_VERSIONS_PER_MODEL>::new()).collect();
         loop {
             let msg = rx.try_recv();
             let no_more_msg = match msg {
@@ -272,27 +272,23 @@ impl<T: Model> PredictService<T> {
                         queue_reset_ts: Instant::now(),
                         queue_earliest_rq_ts: Instant::now(),
                     };
-                    if idx < all_model_predictors.len() {
-                        metrics::NEW_MODEL_SNAPSHOT
-                            .with_label_values(&[&MODEL_SPECS[idx]])
-                            .inc();
+                    assert!(idx < all_model_predictors.len());
+                    metrics::NEW_MODEL_SNAPSHOT
+                        .with_label_values(&[&MODEL_SPECS[idx]])
+                        .inc();
 
+                    //we can do this since the vector is small
+                    let predictors = &mut all_model_predictors[idx];
+                    if predictors.len() == 0 {
+                        info!("now we serve new model: {}", predictor.model);
+                    }
+                    else {
                         info!("now we serve updated model: {}", predictor.model);
-                        //we can do this since the vector is small
-                        let predictors = &mut all_model_predictors[idx];
-                        if predictors.len() == ARGS.versions_per_model {
-                            predictors.remove(predictors.len() - 1);
-                        }
-                        predictors.insert(0, predictor);
-                    } else {
-                        info!("now we serve new model: {:}", predictor.model);
-                        let mut predictors =
-                            ArrayVec::<BatchPredictor<T>, MAX_VERSIONS_PER_MODEL>::new();
-                        predictors.push(predictor);
-                        all_model_predictors.push(predictors);
-                        //check the invariant that we always push the last model to the end
-                        assert_eq!(all_model_predictors.len(), idx + 1)
                     }
+                    if predictors.len() == ARGS.versions_per_model {
+                        predictors.remove(predictors.len() - 1);
+                    }
+                    predictors.insert(0, predictor);
                     false
                 }
                 Err(TryRecvError::Empty) => true,

navi/navi/src/predict_service.rs

@@ -5,39 +5,49 @@ use std::fmt::Display;
  */
 #[derive(Debug)]
 pub enum SegDenseError {
-  IoError(std::io::Error),
-  Json(serde_json::Error),
-  JsonMissingRoot,
-  JsonMissingObject,
-  JsonMissingArray,
-  JsonArraySize,
-  JsonMissingInputFeature,
+    IoError(std::io::Error),
+    Json(serde_json::Error),
+    JsonMissingRoot,
+    JsonMissingObject,
+    JsonMissingArray,
+    JsonArraySize,
+    JsonMissingInputFeature,
 }
 
 impl Display for SegDenseError {
-  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-    match self {
-      SegDenseError::IoError(io_error) => write!(f, "{}", io_error),
-      SegDenseError::Json(serde_json) => write!(f, "{}", serde_json),
-      SegDenseError::JsonMissingRoot => write!(f, "{}", "SegDense JSON: Root Node note found!"),
-      SegDenseError::JsonMissingObject => write!(f, "{}", "SegDense JSON: Object note found!"),
-      SegDenseError::JsonMissingArray => write!(f, "{}", "SegDense JSON: Array Node note found!"),
-      SegDenseError::JsonArraySize => write!(f, "{}", "SegDense JSON: Array size not as expected!"),
-      SegDenseError::JsonMissingInputFeature => write!(f, "{}", "SegDense JSON: Missing input feature!"),
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            SegDenseError::IoError(io_error) => write!(f, "{}", io_error),
+            SegDenseError::Json(serde_json) => write!(f, "{}", serde_json),
+            SegDenseError::JsonMissingRoot => {
+                write!(f, "{}", "SegDense JSON: Root Node note found!")
+            }
+            SegDenseError::JsonMissingObject => {
+                write!(f, "{}", "SegDense JSON: Object note found!")
+            }
+            SegDenseError::JsonMissingArray => {
+                write!(f, "{}", "SegDense JSON: Array Node note found!")
+            }
+            SegDenseError::JsonArraySize => {
+                write!(f, "{}", "SegDense JSON: Array size not as expected!")
+            }
+            SegDenseError::JsonMissingInputFeature => {
+                write!(f, "{}", "SegDense JSON: Missing input feature!")
+            }
+        }
     }
-  }
 }
 
 impl std::error::Error for SegDenseError {}
 
 impl From<std::io::Error> for SegDenseError {
-  fn from(err: std::io::Error) -> Self {
-    SegDenseError::IoError(err)
-  }
+    fn from(err: std::io::Error) -> Self {
+        SegDenseError::IoError(err)
+    }
 }
 
 impl From<serde_json::Error> for SegDenseError {
-  fn from(err: serde_json::Error) -> Self {
-    SegDenseError::Json(err)
-  }
+    fn from(err: serde_json::Error) -> Self {
+        SegDenseError::Json(err)
+    }
 }

navi/segdense/src/error.rs

@@ -1,4 +1,4 @@
 pub mod error;
-pub mod segdense_transform_spec_home_recap_2022; 
 pub mod mapper;
-pub mod util;
+pub mod segdense_transform_spec_home_recap_2022;
+pub mod util;

navi/segdense/src/lib.rs

@@ -5,19 +5,18 @@ use segdense::error::SegDenseError;
 use segdense::util;
 
 fn main() -> Result<(), SegDenseError> {
-  env_logger::init();
-  let args: Vec<String> = env::args().collect();
-  
-  let schema_file_name: &str = if args.len() == 1 {
-    "json/compact.json"
-  } else {
-    &args[1]
-  };
+    env_logger::init();
+    let args: Vec<String> = env::args().collect();
 
-  let json_str = fs::read_to_string(schema_file_name)?;
+    let schema_file_name: &str = if args.len() == 1 {
+        "json/compact.json"
+    } else {
+        &args[1]
+    };
 
-  util::safe_load_config(&json_str)?;
+    let json_str = fs::read_to_string(schema_file_name)?;
 
-  Ok(())
-}
+    util::safe_load_config(&json_str)?;
 
+    Ok(())
+}

navi/segdense/src/main.rs

@@ -19,13 +19,13 @@ pub struct FeatureMapper {
 impl FeatureMapper {
     pub fn new() -> FeatureMapper {
         FeatureMapper {
-            map: HashMap::new()
+            map: HashMap::new(),
         }
     }
 }
 
 pub trait MapWriter {
-    fn set(&mut self, feature_id: i64, info: FeatureInfo); 
+    fn set(&mut self, feature_id: i64, info: FeatureInfo);
 }
 
 pub trait MapReader {

navi/segdense/src/mapper.rs

@@ -164,7 +164,6 @@ pub struct ComplexFeatureTypeTransformSpec {
     pub tensor_shape: Vec<i64>,
 }
 
-
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct InputFeatureMapRecord {

navi/segdense/src/segdense_transform_spec_home_recap_2022.rs

@@ -1,23 +1,23 @@
+use log::debug;
 use std::fs;
-use log::{debug};
 
-use serde_json::{Value, Map};
+use serde_json::{Map, Value};
 
 use crate::error::SegDenseError;
-use crate::mapper::{FeatureMapper, FeatureInfo, MapWriter};
+use crate::mapper::{FeatureInfo, FeatureMapper, MapWriter};
 use crate::segdense_transform_spec_home_recap_2022::{self as seg_dense, InputFeature};
 
-pub fn load_config(file_name: &str) -> seg_dense::Root {
-    let json_str = fs::read_to_string(file_name).expect(
-        &format!("Unable to load segdense file {}", file_name));
-    let seg_dense_config = parse(&json_str).expect(
-        &format!("Unable to parse segdense file {}", file_name));
-    return seg_dense_config;
+pub fn load_config(file_name: &str) -> Result<seg_dense::Root, SegDenseError> {
+    let json_str = fs::read_to_string(file_name)?;
+    // &format!("Unable to load segdense file {}", file_name));
+    let seg_dense_config = parse(&json_str)?;
+    // &format!("Unable to parse segdense file {}", file_name));
+    Ok(seg_dense_config)
 }
 
 pub fn parse(json_str: &str) -> Result<seg_dense::Root, SegDenseError> {
     let root: seg_dense::Root = serde_json::from_str(json_str)?;
-    return Ok(root);
+    Ok(root)
 }
 
 /**
@@ -44,15 +44,8 @@ pub fn safe_load_config(json_str: &str) -> Result<FeatureMapper, SegDenseError>
     load_from_parsed_config(root)
 }
 
-pub fn load_from_parsed_config_ref(root: &seg_dense::Root) -> FeatureMapper {
-    load_from_parsed_config(root.clone()).unwrap_or_else(
-      |error| panic!("Error loading all_config.json - {}", error))
-}
-
 // Perf note : make 'root' un-owned
-pub fn load_from_parsed_config(root: seg_dense::Root) ->
-    Result<FeatureMapper, SegDenseError> {
-
+pub fn load_from_parsed_config(root: seg_dense::Root) -> Result<FeatureMapper, SegDenseError> {
     let v = root.input_features_map;
 
     // Do error check
@@ -86,27 +79,30 @@ pub fn load_from_parsed_config(root: seg_dense::Root) ->
             Some(info) => {
                 debug!("{:?}", info);
                 fm.set(feature_id, info)
-            },
+            }
             None => (),
         }
     }
 
     Ok(fm)
 }
 #[allow(dead_code)]
-fn add_feature_info_to_mapper(feature_mapper: &mut FeatureMapper, input_features: &Vec<InputFeature>) {
+fn add_feature_info_to_mapper(
+    feature_mapper: &mut FeatureMapper,
+    input_features: &Vec<InputFeature>,
+) {
     for input_feature in input_features.iter() {
-            let feature_id = input_feature.feature_id;
-            let feature_info = to_feature_info(input_feature);
-    
-            match feature_info {
-                Some(info) => {
-                    debug!("{:?}", info);
-                    feature_mapper.set(feature_id, info)
-                },
-                None => (),
+        let feature_id = input_feature.feature_id;
+        let feature_info = to_feature_info(input_feature);
+
+        match feature_info {
+            Some(info) => {
+                debug!("{:?}", info);
+                feature_mapper.set(feature_id, info)
             }
+            None => (),
         }
+    }
 }
 
 pub fn to_feature_info(input_feature: &seg_dense::InputFeature) -> Option<FeatureInfo> {
@@ -139,7 +135,7 @@ pub fn to_feature_info(input_feature: &seg_dense::InputFeature) -> Option<Featur
             2 => 0,
             3 => 2,
             _ => -1,
-        }
+        },
     };
 
     if input_feature.index < 0 {
@@ -156,4 +152,3 @@ pub fn to_feature_info(input_feature: &seg_dense::InputFeature) -> Option<Featur
         index_within_tensor: input_feature.index,
     })
 }
-