From 9a8c2a03eb6eed808280b344f6f8c9d45aaed3c3 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Tue, 5 Dec 2023 14:25:36 +0100 Subject: [PATCH 1/3] Don't throw away the right hand time of changes --- src/vcd/signal.rs | 50 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/vcd/signal.rs b/src/vcd/signal.rs index c1522f0..5093dff 100644 --- a/src/vcd/signal.rs +++ b/src/vcd/signal.rs @@ -90,7 +90,7 @@ impl<'a> Signal<'a> { let Signal(signal_enum) = &self; signal_enum .query_string_val_on_tmln(desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals) - .map(|(val, _)| val) + .map(|(val, _, _)| val) } pub fn query_num_val_on_tmln( @@ -101,14 +101,14 @@ impl<'a> Signal<'a> { let Signal(signal_enum) = &self; signal_enum .query_num_val_on_tmln(desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals) - .map(|(val, _)| val) + .map(|(val, _, _)| val) } pub fn query_val_on_tmln( &self, desired_time: &BigUint, vcd: &types::VCD, - ) -> Result<(TimeStamp, SignalValue), SignalErrors> { + ) -> Result<(TimeStamp, SignalValue, Option), SignalErrors> { let Signal(signal_enum) = &self; let num_val = signal_enum.query_num_val_on_tmln( desired_time, @@ -125,15 +125,21 @@ impl<'a> Signal<'a> { // the desired time. If both have valid values, select the most recent // one match (num_val, str_val) { - (Ok((num_val, num_time)), Ok((str_val, str_time))) => { + (Ok((num_val, num_time, num_next)), Ok((str_val, str_time, str_next))) => { + let next = match (num_next, str_next) { + (Some(n), Some(s)) => Some(n.min(s)), + (Some(n), None) => Some(n), + (None, Some(s)) => Some(s), + (None, None) => None + }; if num_time > str_time { - Ok((num_time, SignalValue::BigUint(num_val))) + Ok((num_time, SignalValue::BigUint(num_val), next)) } else { - Ok((str_time, SignalValue::String(str_val))) + Ok((str_time, SignalValue::String(str_val), next)) } } - (Ok((num_val, time)), Err(_)) => Ok((time, SignalValue::BigUint(num_val))), - (Err(_), Ok((str_val, time))) => Ok((time, SignalValue::String(str_val))), + (Ok((num_val, time, next)), Err(_)) => Ok((time, SignalValue::BigUint(num_val), next)), + (Err(_), Ok((str_val, time, next))) => Ok((time, SignalValue::String(str_val), next)), (Err(e), _e) => Err(e), } } @@ -396,7 +402,7 @@ impl SignalEnum { desired_time: &BigUint, tmstmps_encoded_as_u8s: &Vec, all_signals: &Vec, - ) -> Result<(String, TimeStamp), SignalErrors> { + ) -> Result<(String, TimeStamp, Option), SignalErrors> { let signal_idx = match self { Self::Data { self_idx, .. } => { let SignalIdx(idx) = self_idx; @@ -458,7 +464,7 @@ impl SignalEnum { // check if we're requesting a value that occurs beyond the end of the timeline, // if so, return the last value in this timeline if *desired_time > timeline_end_time { - return Ok((timeline_end_val.to_string(), timeline_end_time)); + return Ok((timeline_end_val.to_string(), timeline_end_time, None)); } // This while loop is the meat of the lookup. Performance is log2(n), @@ -477,7 +483,13 @@ impl SignalEnum { lower_idx = mid_idx + 1; } std::cmp::Ordering::Equal => { - return Ok((curr_val.to_string(), curr_time)); + let next_time = if mid_idx >= lsb_indxs_of_string_tmstmp_vals_on_tmln.len() { + Some(self.time_and_str_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) + } + else { + None + }; + return Ok((curr_val.to_string(), curr_time, next_time)); } std::cmp::Ordering::Greater => { upper_idx = mid_idx - 1; @@ -500,14 +512,14 @@ impl SignalEnum { }); } - Ok((left_val.to_string(), left_time)) + Ok((left_val.to_string(), left_time, Some(right_time))) } pub fn query_num_val_on_tmln( &self, desired_time: &BigUint, tmstmps_encoded_as_u8s: &Vec, all_signals: &Vec, - ) -> Result<(BigUint, TimeStamp), SignalErrors> { + ) -> Result<(BigUint, TimeStamp, Option), SignalErrors> { let signal_idx = match self { Self::Data { self_idx, .. } => { let SignalIdx(idx) = self_idx; @@ -589,7 +601,7 @@ impl SignalEnum { // check if we're requesting a value that occurs beyond the end of the timeline, // if so, return the last value in this timeline if *desired_time > timeline_end_time { - return Ok((timeline_end_val, timeline_end_time)); + return Ok((timeline_end_val, timeline_end_time, None)); } // This while loop is the meat of the lookup. Performance is log2(n), @@ -608,7 +620,13 @@ impl SignalEnum { lower_idx = mid_idx + 1; } std::cmp::Ordering::Equal => { - return Ok((curr_val, curr_time)); + let next_time = if mid_idx >= lsb_indxs_of_num_tmstmp_vals_on_tmln.len() { + Some(self.time_and_num_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) + } + else { + None + }; + return Ok((curr_val, curr_time, next_time)); } std::cmp::Ordering::Greater => { upper_idx = mid_idx - 1; @@ -631,6 +649,6 @@ impl SignalEnum { }); } - Ok((left_val, left_time)) + Ok((left_val, left_time, Some(right_time))) } } From 7d414f36ddc8817a640787c3279b71cf36b54938 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Tue, 5 Dec 2023 14:59:04 +0100 Subject: [PATCH 2/3] Correctly compute next index --- src/vcd/signal.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vcd/signal.rs b/src/vcd/signal.rs index 5093dff..7d0cfc4 100644 --- a/src/vcd/signal.rs +++ b/src/vcd/signal.rs @@ -483,7 +483,7 @@ impl SignalEnum { lower_idx = mid_idx + 1; } std::cmp::Ordering::Equal => { - let next_time = if mid_idx >= lsb_indxs_of_string_tmstmp_vals_on_tmln.len() { + let next_time = if mid_idx < lsb_indxs_of_string_tmstmp_vals_on_tmln.len()-1 { Some(self.time_and_str_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) } else { @@ -620,7 +620,7 @@ impl SignalEnum { lower_idx = mid_idx + 1; } std::cmp::Ordering::Equal => { - let next_time = if mid_idx >= lsb_indxs_of_num_tmstmp_vals_on_tmln.len() { + let next_time = if mid_idx < lsb_indxs_of_num_tmstmp_vals_on_tmln.len() - 1 { Some(self.time_and_num_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) } else { From e2e3541e3f5632be33a69f646293f47c2ea8e2c0 Mon Sep 17 00:00:00 2001 From: TheZoq2 Date: Tue, 5 Dec 2023 16:44:19 +0100 Subject: [PATCH 3/3] Refactor the double-sided-query to make it less error-prone --- src/vcd/signal.rs | 147 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 104 insertions(+), 43 deletions(-) diff --git a/src/vcd/signal.rs b/src/vcd/signal.rs index 7d0cfc4..b87e1a2 100644 --- a/src/vcd/signal.rs +++ b/src/vcd/signal.rs @@ -40,6 +40,11 @@ pub enum SignalValue { String(String), } +pub struct QueryResult { + pub current: Option<(TimeStamp, T)>, + pub next: Option, +} + pub struct Signal<'a>(pub(super) &'a SignalEnum); impl<'a> Signal<'a> { @@ -82,6 +87,9 @@ impl<'a> Signal<'a> { signal_enum.bits_required() } + // NOTE: (zoq) I am removing thse because they aren't used in Surfer so I can't test them + // properly + /* pub fn query_string_val_on_tmln( &self, desired_time: &BigUint, @@ -90,32 +98,33 @@ impl<'a> Signal<'a> { let Signal(signal_enum) = &self; signal_enum .query_string_val_on_tmln(desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals) - .map(|(val, _, _)| val) + .map(|QueryResult{current, next: _}| current.map(|c| c.1)) } pub fn query_num_val_on_tmln( &self, desired_time: &BigUint, vcd: &types::VCD, - ) -> Result { + ) -> Result, SignalErrors> { let Signal(signal_enum) = &self; signal_enum .query_num_val_on_tmln(desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals) - .map(|(val, _, _)| val) + .map(|QueryResult{current, next: _}| current.map(|c| c.1)) } + */ pub fn query_val_on_tmln( &self, desired_time: &BigUint, vcd: &types::VCD, - ) -> Result<(TimeStamp, SignalValue, Option), SignalErrors> { + ) -> Result, SignalErrors> { let Signal(signal_enum) = &self; - let num_val = signal_enum.query_num_val_on_tmln( + let num_query_out = signal_enum.query_num_val_on_tmln( desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals, ); - let str_val = signal_enum.query_string_val_on_tmln( + let str_query_out = signal_enum.query_string_val_on_tmln( desired_time, &vcd.tmstmps_encoded_as_u8s, &vcd.all_signals, @@ -124,22 +133,44 @@ impl<'a> Signal<'a> { // Both num and str will return the newest value that is closest to // the desired time. If both have valid values, select the most recent // one - match (num_val, str_val) { - (Ok((num_val, num_time, num_next)), Ok((str_val, str_time, str_next))) => { - let next = match (num_next, str_next) { + match (num_query_out, str_query_out) { + (Ok(num_result), Ok(str_result)) => { + let next = match (num_result.next, str_result.next) { (Some(n), Some(s)) => Some(n.min(s)), (Some(n), None) => Some(n), (None, Some(s)) => Some(s), - (None, None) => None + (None, None) => None, }; - if num_time > str_time { - Ok((num_time, SignalValue::BigUint(num_val), next)) - } else { - Ok((str_time, SignalValue::String(str_val), next)) + + match (num_result.current, str_result.current) { + (Some((num_time, num_value)), Some((str_time, str_value))) => { + if num_time > str_time { + Ok(QueryResult { + current: Some((num_time, SignalValue::BigUint(num_value))), + next, + }) + } else { + Ok(QueryResult { + current: Some((str_time, SignalValue::String(str_value))), + next, + }) + } + } + (Some((num_time, num_val)), None) => Ok(QueryResult { + current: Some((num_time, SignalValue::BigUint(num_val))), + next, + }), + (None, Some((str_time, str_value))) => Ok(QueryResult { + current: Some((str_time, SignalValue::String(str_value))), + next, + }), + (None, None) => Ok(QueryResult { + current: None, + next, + }), } } - (Ok((num_val, time, next)), Err(_)) => Ok((time, SignalValue::BigUint(num_val), next)), - (Err(_), Ok((str_val, time, next))) => Ok((time, SignalValue::String(str_val), next)), + (_e, Err(e)) => Err(e), (Err(e), _e) => Err(e), } } @@ -198,10 +229,6 @@ pub(super) enum SignalEnum { #[derive(Debug)] pub enum SignalErrors { - PreTimeline { - desired_time: BigUint, - timeline_start_time: BigUint, - }, EmptyTimeline, TimelineNotMultiple, StrTmlnLenMismatch, @@ -402,7 +429,7 @@ impl SignalEnum { desired_time: &BigUint, tmstmps_encoded_as_u8s: &Vec, all_signals: &Vec, - ) -> Result<(String, TimeStamp, Option), SignalErrors> { + ) -> Result, SignalErrors> { let signal_idx = match self { Self::Data { self_idx, .. } => { let SignalIdx(idx) = self_idx; @@ -436,7 +463,10 @@ impl SignalEnum { // this signal should at least have some events, otherwise, trying to index into // an empty vector later on would fail if lsb_indxs_of_string_tmstmp_vals_on_tmln.is_empty() { - return Err(SignalErrors::EmptyTimeline); + return Ok(QueryResult { + current: None, + next: None + }); } // the vector of string timeline lsb indices should have the same @@ -450,9 +480,9 @@ impl SignalEnum { let (timeline_start_time, _) = self.time_and_str_val_at_event_idx(0, tmstmps_encoded_as_u8s)?; if *desired_time < timeline_start_time { - return Err(SignalErrors::PreTimeline { - desired_time: desired_time.clone(), - timeline_start_time, + return Ok(QueryResult { + current: None, + next: Some(timeline_start_time), }); } @@ -464,7 +494,10 @@ impl SignalEnum { // check if we're requesting a value that occurs beyond the end of the timeline, // if so, return the last value in this timeline if *desired_time > timeline_end_time { - return Ok((timeline_end_val.to_string(), timeline_end_time, None)); + return Ok(QueryResult { + current: Some((timeline_end_time, timeline_end_val.to_string())), + next: None, + }); } // This while loop is the meat of the lookup. Performance is log2(n), @@ -483,13 +516,21 @@ impl SignalEnum { lower_idx = mid_idx + 1; } std::cmp::Ordering::Equal => { - let next_time = if mid_idx < lsb_indxs_of_string_tmstmp_vals_on_tmln.len()-1 { - Some(self.time_and_str_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) - } - else { + let next_time = if mid_idx < lsb_indxs_of_string_tmstmp_vals_on_tmln.len() - 1 { + Some( + self.time_and_str_val_at_event_idx( + mid_idx + 1, + tmstmps_encoded_as_u8s, + )? + .0, + ) + } else { None }; - return Ok((curr_val.to_string(), curr_time, next_time)); + return Ok(QueryResult { + current: Some((curr_time, curr_val.to_string())), + next: next_time, + }); } std::cmp::Ordering::Greater => { upper_idx = mid_idx - 1; @@ -512,14 +553,17 @@ impl SignalEnum { }); } - Ok((left_val.to_string(), left_time, Some(right_time))) + Ok(QueryResult { + current: Some((left_time, left_val.to_string())), + next: Some(right_time), + }) } pub fn query_num_val_on_tmln( &self, desired_time: &BigUint, tmstmps_encoded_as_u8s: &Vec, all_signals: &Vec, - ) -> Result<(BigUint, TimeStamp, Option), SignalErrors> { + ) -> Result, SignalErrors> { let signal_idx = match self { Self::Data { self_idx, .. } => { let SignalIdx(idx) = self_idx; @@ -564,7 +608,10 @@ impl SignalEnum { // this signal should at least have some events, otherwise, trying to index into // an empty vector later on would fail if lsb_indxs_of_num_tmstmp_vals_on_tmln.is_empty() { - return Err(SignalErrors::EmptyTimeline); + return Ok(QueryResult { + current: None, + next: None + }); } // assertion that value_sequence is a proper multiple of @@ -587,9 +634,9 @@ impl SignalEnum { let (timeline_start_time, _) = self.time_and_num_val_at_event_idx(0, tmstmps_encoded_as_u8s)?; if *desired_time < timeline_start_time { - return Err(SignalErrors::PreTimeline { - desired_time: desired_time.clone(), - timeline_start_time, + return Ok(QueryResult { + current: None, + next: Some(timeline_start_time), }); } @@ -601,7 +648,10 @@ impl SignalEnum { // check if we're requesting a value that occurs beyond the end of the timeline, // if so, return the last value in this timeline if *desired_time > timeline_end_time { - return Ok((timeline_end_val, timeline_end_time, None)); + return Ok(QueryResult { + current: Some((timeline_end_time, timeline_end_val)), + next: None, + }); } // This while loop is the meat of the lookup. Performance is log2(n), @@ -621,12 +671,20 @@ impl SignalEnum { } std::cmp::Ordering::Equal => { let next_time = if mid_idx < lsb_indxs_of_num_tmstmp_vals_on_tmln.len() - 1 { - Some(self.time_and_num_val_at_event_idx(mid_idx+1, tmstmps_encoded_as_u8s)?.0) - } - else { + Some( + self.time_and_num_val_at_event_idx( + mid_idx + 1, + tmstmps_encoded_as_u8s, + )? + .0, + ) + } else { None }; - return Ok((curr_val, curr_time, next_time)); + return Ok(QueryResult { + current: Some((curr_time, curr_val)), + next: next_time, + }); } std::cmp::Ordering::Greater => { upper_idx = mid_idx - 1; @@ -649,6 +707,9 @@ impl SignalEnum { }); } - Ok((left_val, left_time, Some(right_time))) + return Ok(QueryResult { + current: Some((left_time, left_val)), + next: Some(right_time), + }); } }