From 4627d6017edf3474214db0094a59d1e431374af8 Mon Sep 17 00:00:00 2001 From: "Joshua A. Anderson" Date: Mon, 13 May 2024 15:00:20 -0400 Subject: [PATCH] Pre-commit and start implement >= <= --- doc/src/SUMMARY.md | 2 +- doc/src/workflow/action/index.md | 2 +- src/expr.rs | 55 +++++++++- src/project.rs | 2 +- src/scheduler/bash.rs | 17 +-- src/workflow.rs | 171 +++++++++++++++++++++---------- 6 files changed, 181 insertions(+), 68 deletions(-) diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 946a43e..c831795 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -25,7 +25,7 @@ - [Advanced topics]() - [Submit the same action to different groups/resources]() - [Use an action to summarize many directories]() - + # Reference - [row](row/index.md) diff --git a/doc/src/workflow/action/index.md b/doc/src/workflow/action/index.md index e462d49..b8807db 100644 --- a/doc/src/workflow/action/index.md +++ b/doc/src/workflow/action/index.md @@ -29,7 +29,7 @@ walltime.per_submission = "04:00:00" action, which may be set by [from](#from). > Note: Two (or more) conceptually identical elements in the actions array may have -> the same name. All elements with the same name **must** have identical +> the same name. All elements with the same name **must** have identical > [`products`](#products) and [`previous_actions`](#previous_actions). All elements > with the same name **must also** select non-intersecting subsets of directories with > [`group.include`](group.md#include). diff --git a/src/expr.rs b/src/expr.rs index 0b76aed..a6fbb70 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -61,15 +61,14 @@ pub(crate) fn evaluate_json_comparison( ) -> Option { #[allow(clippy::match_same_arms)] match (comparison, partial_cmp_json_values(a, b)) { + (Comparison::LessThan, Some(Ordering::Less)) => Some(true), + (Comparison::LessThanOrEqualTo, Some(Ordering::Less | Ordering::Equal)) => Some(true), (Comparison::EqualTo, Some(Ordering::Equal)) => Some(true), + (Comparison::GreaterThanOrEqualTo, Some(Ordering::Greater | Ordering::Equal)) => Some(true), (Comparison::GreaterThan, Some(Ordering::Greater)) => Some(true), - (Comparison::LessThan, Some(Ordering::Less)) => Some(true), (_, None) => None, (_, _) => Some(false), } - -// TODO: Greater than or equal to -// TODO: Less than or equal to } #[cfg(test)] @@ -166,6 +165,22 @@ mod tests { evaluate_json_comparison(&Comparison::EqualTo, &Value::from(5), &Value::from(5)), Some(true) ); + assert_eq!( + evaluate_json_comparison( + &Comparison::GreaterThanOrEqualTo, + &Value::from(5), + &Value::from(5) + ), + Some(true) + ); + assert_eq!( + evaluate_json_comparison( + &Comparison::LessThanOrEqualTo, + &Value::from(5), + &Value::from(5) + ), + Some(true) + ); assert_eq!( evaluate_json_comparison(&Comparison::EqualTo, &Value::from(5), &Value::from(10)), Some(false) @@ -174,9 +189,41 @@ mod tests { evaluate_json_comparison(&Comparison::GreaterThan, &Value::from(5), &Value::from(10)), Some(false) ); + assert_eq!( + evaluate_json_comparison( + &Comparison::GreaterThanOrEqualTo, + &Value::from(5), + &Value::from(10) + ), + Some(false) + ); + assert_eq!( + evaluate_json_comparison( + &Comparison::GreaterThanOrEqualTo, + &Value::from(6), + &Value::from(5) + ), + Some(true) + ); assert_eq!( evaluate_json_comparison(&Comparison::LessThan, &Value::from(5), &Value::from(10)), Some(true) ); + assert_eq!( + evaluate_json_comparison( + &Comparison::LessThanOrEqualTo, + &Value::from(5), + &Value::from(10) + ), + Some(true) + ); + assert_eq!( + evaluate_json_comparison( + &Comparison::LessThanOrEqualTo, + &Value::from(5), + &Value::from(4) + ), + Some(false) + ); } } diff --git a/src/project.rs b/src/project.rs index 1f8ebfe..b43e535 100644 --- a/src/project.rs +++ b/src/project.rs @@ -465,7 +465,7 @@ previous_actions = ["two"] ); let mut action = project.workflow.action[1].clone(); - let include = &mut action.group.include.as_mut().unwrap(); + let include = action.group.include.as_mut().unwrap(); include.push(("/i".into(), Comparison::GreaterThan, Value::from(4))); assert_eq!( project diff --git a/src/scheduler/bash.rs b/src/scheduler/bash.rs index 545d16c..e9f011a 100644 --- a/src/scheduler/bash.rs +++ b/src/scheduler/bash.rs @@ -96,10 +96,14 @@ export ACTION_NAME="{}" export ACTION_PROCESSES="{}" export ACTION_WALLTIME_IN_MINUTES="{}" "#, - self.cluster_name, self.action.name(), self.total_processes, self.walltime_in_minutes, + self.cluster_name, + self.action.name(), + self.total_processes, + self.walltime_in_minutes, ); - if let Processes::PerDirectory(processes_per_directory) = self.action.resources.processes() { + if let Processes::PerDirectory(processes_per_directory) = self.action.resources.processes() + { let _ = writeln!( result, "export ACTION_PROCESSES_PER_DIRECTORY=\"{processes_per_directory}\"", @@ -322,8 +326,8 @@ mod tests { threads_per_process: Some(4), gpus_per_process: Some(1), walltime: Some(Walltime::PerSubmission( - Duration::new(true, 0, 240, 0).expect("Valid duration.")), - ), + Duration::new(true, 0, 240, 0).expect("Valid duration."), + )), }; let action = Action { @@ -505,8 +509,9 @@ mod tests { fn more_variables() { let (mut action, directories, launchers) = setup(); action.resources.processes = Some(Processes::PerSubmission(10)); - action.resources.walltime = - Some(Walltime::PerDirectory(Duration::new(true, 0, 60, 0).expect("Valid duration."))); + action.resources.walltime = Some(Walltime::PerDirectory( + Duration::new(true, 0, 60, 0).expect("Valid duration."), + )); action.resources.threads_per_process = None; action.resources.gpus_per_process = None; diff --git a/src/workflow.rs b/src/workflow.rs index fc8b1bd..7196c37 100644 --- a/src/workflow.rs +++ b/src/workflow.rs @@ -131,7 +131,6 @@ pub struct DefaultTables { pub action: Action, } - #[derive(Clone, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum Walltime { @@ -170,7 +169,9 @@ pub struct Resources { #[serde(rename_all = "snake_case")] pub enum Comparison { LessThan, + LessThanOrEqualTo, EqualTo, + GreaterThanOrEqualTo, GreaterThan, } // TODO: Possible to use <, >, <=, >=, ==? @@ -398,7 +399,7 @@ impl Resources { } impl Action { - /// Get the action's name + /// Get the action's `name`. pub fn name(&self) -> &str { if let Some(name) = self.name.as_ref() { name @@ -407,7 +408,7 @@ impl Action { } } - /// Get the action's command + /// Get the action's `command`. pub fn command(&self) -> &str { if let Some(command) = self.command.as_ref() { command @@ -416,7 +417,7 @@ impl Action { } } - /// Get the action's launchers + /// Get the action's `launchers`. pub fn launchers(&self) -> &[String] { if let Some(launchers) = self.launchers.as_ref() { launchers @@ -425,7 +426,7 @@ impl Action { } } - /// Get the action's previous_actions + /// Get the action's `previous_actions`. pub fn previous_actions(&self) -> &[String] { if let Some(previous_actions) = self.previous_actions.as_ref() { previous_actions @@ -445,7 +446,6 @@ impl Action { /// Resolve the action's omitted keys with defaults fn resolve(&mut self, template: &Action) { - if self.name.is_none() { self.name = template.name.clone(); } @@ -454,7 +454,7 @@ impl Action { } if self.launchers.is_none() { self.launchers = template.launchers.clone(); - } + } if self.previous_actions.is_none() { self.previous_actions = template.previous_actions.clone(); } @@ -463,7 +463,7 @@ impl Action { } self.resources.resolve(&template.resources); - self.group.resolve(&template.group); + self.group.resolve(&template.group); // Populate each action's submit_options with the global ones. for (name, template_options) in &template.submit_options { @@ -485,16 +485,15 @@ impl Action { action_options.custom = template_options.custom.clone(); } } else { - self - .submit_options + self.submit_options .insert(name.clone(), template_options.clone()); } - } + } } } impl Group { - /// Get the group's include. + /// Get the group's `include`. pub fn include(&self) -> &[(String, Comparison, serde_json::Value)] { if let Some(include) = self.include.as_ref() { include @@ -503,7 +502,7 @@ impl Group { } } - /// Get the group's sort_by. + /// Get the group's `sort_by`. pub fn sort_by(&self) -> &[String] { if let Some(sort_by) = self.sort_by.as_ref() { sort_by @@ -512,7 +511,7 @@ impl Group { } } - /// Get the group's split_by_sort_key. + /// Get the group's `split_by_sort_key`. pub fn split_by_sort_key(&self) -> bool { if let Some(split_by_sort_key) = self.split_by_sort_key { split_by_sort_key @@ -521,7 +520,7 @@ impl Group { } } - /// Get the group's reverse_sort. + /// Get the group's `reverse_sort`. pub fn reverse_sort(&self) -> bool { if let Some(reverse_sort) = self.reverse_sort { reverse_sort @@ -530,7 +529,7 @@ impl Group { } } - /// Get the group's submit_whole. + /// Get the group's `submit_whole`. pub fn submit_whole(&self) -> bool { if let Some(submit_whole) = self.submit_whole { submit_whole @@ -560,7 +559,6 @@ impl Group { self.submit_whole = template.submit_whole; } } - } impl Workflow { @@ -623,30 +621,29 @@ impl Workflow { } let source_actions = self.action.clone(); - + for (action_idx, action) in self.action.iter_mut().enumerate() { - if let Some(from) = &action.from { if let Some(action_index) = source_actions.iter().position(|a| a.name() == from) { if let Some(recursive_from) = &source_actions[action_index].from { return Err(Error::RecursiveFrom(recursive_from.clone())); } - + action.resolve(&source_actions[action_index]); } else { return Err(Error::FromActionNotFound(from.clone())); } } - + action.resolve(&self.default.action); action_names.insert(action.name().to_string()); trace!("Validating action '{}'.", action.name()); - if action.name == None { + if action.name.is_none() { return Err(Error::ActionMissingName(action_idx)); } - if action.command == None { + if action.command.is_none() { return Err(Error::ActionMissingCommand(action.name().into())); } @@ -668,12 +665,16 @@ impl Workflow { } } - if let Some(first_action) = self.action_by_name(&action.name()) { + if let Some(first_action) = self.action_by_name(action.name()) { if action.previous_actions != first_action.previous_actions { - return Err(Error::DuplicateActionsDifferentPreviousActions(action.name().to_string())); + return Err(Error::DuplicateActionsDifferentPreviousActions( + action.name().to_string(), + )); } if action.products != first_action.products { - return Err(Error::DuplicateActionsDifferentProducts(action.name().to_string())); + return Err(Error::DuplicateActionsDifferentProducts( + action.name().to_string(), + )); } } } @@ -899,10 +900,7 @@ command = "c" assert_eq!(action.resources.processes(), Processes::PerSubmission(1)); assert_eq!(action.resources.threads_per_process, None); assert_eq!(action.resources.gpus_per_process, None); - assert_eq!( - action.resources.walltime, - None, - ); + assert_eq!(action.resources.walltime, None,); assert_eq!( action.resources.walltime(), Walltime::PerDirectory(Duration::new(true, 0, 3600, 0).unwrap()) @@ -935,7 +933,7 @@ command = "c" assert!(result.unwrap_err().to_string().contains("missing `name`")); } - + #[test] #[parallel] fn action_no_command() { @@ -947,9 +945,12 @@ name = "a" let result = Workflow::open_str(temp.path(), workflow); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("missing `command`")); + assert!(result + .unwrap_err() + .to_string() + .contains("missing `command`")); } - + #[test] #[parallel] fn group_defaults() { @@ -1008,9 +1009,15 @@ command = "d" products = ["b"] "#; let result = Workflow::open_str(temp.path(), workflow); - assert!(matches!(result, Err(Error::DuplicateActionsDifferentProducts(_)))); + assert!(matches!( + result, + Err(Error::DuplicateActionsDifferentProducts(_)) + )); - assert!(result.unwrap_err().to_string().contains("must have the same `products`")); + assert!(result + .unwrap_err() + .to_string() + .contains("must have the same `products`")); } #[test] @@ -1032,9 +1039,15 @@ name = "a" command = "e" "#; let result = Workflow::open_str(temp.path(), workflow); - assert!(matches!(result, Err(Error::DuplicateActionsDifferentPreviousActions(_)))); + assert!(matches!( + result, + Err(Error::DuplicateActionsDifferentPreviousActions(_)) + )); - assert!(result.unwrap_err().to_string().contains("must have the same `previous_actions`")); + assert!(result + .unwrap_err() + .to_string() + .contains("must have the same `previous_actions`")); } #[test] @@ -1053,7 +1066,10 @@ launchers = ["openmp", "mpi"] assert_eq!(workflow.action.len(), 1); let action = workflow.action.first().unwrap(); - assert_eq!(action.launchers(), vec!["openmp".to_string(), "mpi".to_string()]); + assert_eq!( + action.launchers(), + vec!["openmp".to_string(), "mpi".to_string()] + ); } #[test] @@ -1484,16 +1500,19 @@ from = "a" let result = Workflow::open_str(temp.path(), workflow); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("must not set `from`")); + assert!(result + .unwrap_err() + .to_string() + .contains("must not set `from`")); } #[test] #[parallel] fn empty_action_default() { let temp = TempDir::new().unwrap(); - let workflow = r#" + let workflow = " [default.action] -"#; +"; let workflow = Workflow::open_str(temp.path(), workflow).unwrap(); @@ -1566,9 +1585,15 @@ name = "d" assert_eq!(action.resources.processes(), Processes::PerDirectory(2)); assert_eq!(action.resources.threads_per_process, Some(3)); assert_eq!(action.resources.gpus_per_process, Some(4)); - assert_eq!(action.resources.walltime(), Walltime::PerSubmission(Duration::new(true, 0, 1, 0).unwrap())); + assert_eq!( + action.resources.walltime(), + Walltime::PerSubmission(Duration::new(true, 0, 1, 0).unwrap()) + ); assert!(action.submit_options.is_empty()); - assert_eq!(action.group.include(), vec![("/f".into(), Comparison::EqualTo, serde_json::Value::from(5))]); + assert_eq!( + action.group.include(), + vec![("/f".into(), Comparison::EqualTo, serde_json::Value::from(5))] + ); assert_eq!(action.group.sort_by(), vec!["/g"]); assert!(action.group.split_by_sort_key()); assert!(action.group.reverse_sort()); @@ -1576,7 +1601,7 @@ name = "d" assert!(action.group.submit_whole()); assert_eq!(action.from, None); } - + #[test] #[parallel] fn action_override_default() { @@ -1644,9 +1669,19 @@ name = "dd" assert_eq!(action.resources.processes(), Processes::PerDirectory(4)); assert_eq!(action.resources.threads_per_process, Some(6)); assert_eq!(action.resources.gpus_per_process, Some(8)); - assert_eq!(action.resources.walltime(), Walltime::PerSubmission(Duration::new(true, 0, 2, 0).unwrap())); + assert_eq!( + action.resources.walltime(), + Walltime::PerSubmission(Duration::new(true, 0, 2, 0).unwrap()) + ); assert!(action.submit_options.is_empty()); - assert_eq!(action.group.include(), vec![("/ff".into(), Comparison::EqualTo, serde_json::Value::from(10))]); + assert_eq!( + action.group.include(), + vec![( + "/ff".into(), + Comparison::EqualTo, + serde_json::Value::from(10) + )] + ); assert_eq!(action.group.sort_by(), vec!["/gg"]); assert!(!action.group.split_by_sort_key()); assert!(!action.group.reverse_sort()); @@ -1704,9 +1739,15 @@ command = "e" assert_eq!(action.resources.processes(), Processes::PerDirectory(2)); assert_eq!(action.resources.threads_per_process, Some(3)); assert_eq!(action.resources.gpus_per_process, Some(4)); - assert_eq!(action.resources.walltime(), Walltime::PerSubmission(Duration::new(true, 0, 1, 0).unwrap())); + assert_eq!( + action.resources.walltime(), + Walltime::PerSubmission(Duration::new(true, 0, 1, 0).unwrap()) + ); assert!(action.submit_options.is_empty()); - assert_eq!(action.group.include(), vec![("/f".into(), Comparison::EqualTo, serde_json::Value::from(5))]); + assert_eq!( + action.group.include(), + vec![("/f".into(), Comparison::EqualTo, serde_json::Value::from(5))] + ); assert_eq!(action.group.sort_by(), vec!["/g"]); assert!(action.group.split_by_sort_key()); assert!(action.group.reverse_sort()); @@ -1790,9 +1831,19 @@ command = "e" assert_eq!(action.resources.processes(), Processes::PerDirectory(4)); assert_eq!(action.resources.threads_per_process, Some(6)); assert_eq!(action.resources.gpus_per_process, Some(8)); - assert_eq!(action.resources.walltime(), Walltime::PerSubmission(Duration::new(true, 0, 2, 0).unwrap())); + assert_eq!( + action.resources.walltime(), + Walltime::PerSubmission(Duration::new(true, 0, 2, 0).unwrap()) + ); assert!(action.submit_options.is_empty()); - assert_eq!(action.group.include(), vec![("/ff".into(), Comparison::EqualTo, serde_json::Value::from(10))]); + assert_eq!( + action.group.include(), + vec![( + "/ff".into(), + Comparison::EqualTo, + serde_json::Value::from(10) + )] + ); assert_eq!(action.group.sort_by(), vec!["/gg"]); assert!(!action.group.split_by_sort_key()); assert!(!action.group.reverse_sort()); @@ -1905,7 +1956,9 @@ resources.processes.per_directory = 8 #[parallel] fn total_walltime() { let r = Resources { - walltime: Some(Walltime::PerDirectory(Duration::new(true, 1, 3600, 0).unwrap())), + walltime: Some(Walltime::PerDirectory( + Duration::new(true, 1, 3600, 0).unwrap(), + )), ..Resources::default() }; @@ -1923,7 +1976,9 @@ resources.processes.per_directory = 8 ); let r = Resources { - walltime: Some(Walltime::PerSubmission(Duration::new(true, 1, 3600, 0).unwrap())), + walltime: Some(Walltime::PerSubmission( + Duration::new(true, 1, 3600, 0).unwrap(), + )), ..Resources::default() }; @@ -1946,7 +2001,9 @@ resources.processes.per_directory = 8 fn resource_cost() { let r = Resources { processes: Some(Processes::PerSubmission(10)), - walltime: Some(Walltime::PerDirectory(Duration::new(true, 0, 3600, 0).unwrap())), + walltime: Some(Walltime::PerDirectory( + Duration::new(true, 0, 3600, 0).unwrap(), + )), ..Resources::default() }; @@ -1956,7 +2013,9 @@ resources.processes.per_directory = 8 let r = Resources { processes: Some(Processes::PerSubmission(10)), - walltime: Some(Walltime::PerDirectory(Duration::new(true, 0, 3600, 0).unwrap())), + walltime: Some(Walltime::PerDirectory( + Duration::new(true, 0, 3600, 0).unwrap(), + )), threads_per_process: Some(4), ..Resources::default() }; @@ -1967,7 +2026,9 @@ resources.processes.per_directory = 8 let r = Resources { processes: Some(Processes::PerSubmission(10)), - walltime: Some(Walltime::PerDirectory(Duration::new(true, 0, 3600, 0).unwrap())), + walltime: Some(Walltime::PerDirectory( + Duration::new(true, 0, 3600, 0).unwrap(), + )), threads_per_process: Some(4), gpus_per_process: Some(2), };