From 4d636fc1a90523267c0f61c6de8cec0e43ece0cb Mon Sep 17 00:00:00 2001 From: carl-drews Date: Thu, 12 Sep 2024 16:16:09 -0600 Subject: [PATCH 01/11] Adding check for duplicate MUSICA keys. --- src/acom_music_box/main.py | 1 + src/acom_music_box/music_box.py | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/acom_music_box/main.py b/src/acom_music_box/main.py index 9e1c7f8e..782c9fba 100644 --- a/src/acom_music_box/main.py +++ b/src/acom_music_box/main.py @@ -158,6 +158,7 @@ def main(): os.path.dirname(musicBoxConfigFile), myBox.config_file) myBox.create_solver(config_path) + myBox.check_config(config_path) result = myBox.solve(musicBoxOutputPath) if musicBoxOutputPath is None: diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index be34c57e..19241a69 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -89,6 +89,45 @@ def create_solver( solver_type, number_of_grid_cells) + def check_config(self, boxConfigPath): + """ + Verifies correct configuration of the solver object. + + Args: + boxConfigPath = filename and path of MusicBox configuration file + + Returns: + True if all checks passed + Throws error for the first check failed. + """ + if (not self.solver): + return(False) + + # look for duplicate reaction names + if (self.initial_conditions): + if (self.initial_conditions.reaction_rates): + reactionNames = [] + for rate in self.initial_conditions.reaction_rates: + # watch out for Nones in here + if not rate.reaction: + continue + if not rate.reaction.name: + continue + reactionNames.append(rate.reaction.name) + + # look for name already seen + seen = set() + dupNames = [name for name in reactionNames if name in seen or seen.add(name)] + + if (len(dupNames) > 0): + # inform user of the error and its remedy + errString = ("Error: Duplicate reaction names specified within {}: {}." + .format(boxConfigPath, dupNames)) + errString += " Please remove or rename the duplicates." + raise Exception(errString) + + return(True) + def solve(self, output_path=None, callback=None): """ Solves the box model simulation and optionally writes the output to a file. @@ -260,6 +299,7 @@ def readConditionsFromJson(self, path_to_json): ValueError: If the JSON string cannot be parsed. """ + #self.config_file = path_to_json # bogus with open(path_to_json, 'r') as json_file: data = json.load(json_file) # Set box model options @@ -307,6 +347,8 @@ def order_reaction_rates(self, curr_conditions, rate_constant_ordering): Returns: list: An ordered list of rate constants. """ + # look through the rates for duplicate reaction names + rate_constants = {} for rate in curr_conditions.reaction_rates: @@ -321,7 +363,10 @@ def order_reaction_rates(self, curr_conditions, rate_constant_ordering): rate_constants[key] = rate.rate ordered_rate_constants = len(rate_constants.keys()) * [0.0] + #ordered_rate_constants = (len(rate_constants.keys()) + 10) * [0.0] + logger.info("len orc = {}".format(len(ordered_rate_constants))) for key, value in rate_constants.items(): + logger.info("key = {} index = {}".format(key, rate_constant_ordering[key])) ordered_rate_constants[rate_constant_ordering[key]] = float(value) return ordered_rate_constants From 62bb909e685102a5ea567d075fe15a5a3e9b03f0 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Thu, 12 Sep 2024 16:22:59 -0600 Subject: [PATCH 02/11] Removed diagnosic code. --- src/acom_music_box/music_box.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index 19241a69..0d8b2f5d 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -299,7 +299,6 @@ def readConditionsFromJson(self, path_to_json): ValueError: If the JSON string cannot be parsed. """ - #self.config_file = path_to_json # bogus with open(path_to_json, 'r') as json_file: data = json.load(json_file) # Set box model options @@ -347,8 +346,6 @@ def order_reaction_rates(self, curr_conditions, rate_constant_ordering): Returns: list: An ordered list of rate constants. """ - # look through the rates for duplicate reaction names - rate_constants = {} for rate in curr_conditions.reaction_rates: @@ -363,10 +360,7 @@ def order_reaction_rates(self, curr_conditions, rate_constant_ordering): rate_constants[key] = rate.rate ordered_rate_constants = len(rate_constants.keys()) * [0.0] - #ordered_rate_constants = (len(rate_constants.keys()) + 10) * [0.0] - logger.info("len orc = {}".format(len(ordered_rate_constants))) for key, value in rate_constants.items(): - logger.info("key = {} index = {}".format(key, rate_constant_ordering[key])) ordered_rate_constants[rate_constant_ordering[key]] = float(value) return ordered_rate_constants From ff7a263749853f9d7a2698b0e6ca4cd284ec7073 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Sun, 15 Sep 2024 19:11:38 -0600 Subject: [PATCH 03/11] Added stub for unit test. --- src/acom_music_box/music_box.py | 2 ++ tests/integration/test_duplicate_reactions.py | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/integration/test_duplicate_reactions.py diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index 0d8b2f5d..6d882ca1 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -95,6 +95,8 @@ def check_config(self, boxConfigPath): Args: boxConfigPath = filename and path of MusicBox configuration file + This filename is supplied only for the error message; + the configuration should already be loaded. Returns: True if all checks passed diff --git a/tests/integration/test_duplicate_reactions.py b/tests/integration/test_duplicate_reactions.py new file mode 100644 index 00000000..7e54b039 --- /dev/null +++ b/tests/integration/test_duplicate_reactions.py @@ -0,0 +1,18 @@ +from acom_music_box import MusicBox, Examples + +import os +import csv +import math + + +class TestDuplicateReactions: + def test_run(self): + box_model = MusicBox() + + assert True, f"All is good." + + +if __name__ == "__main__": + test = TestDuplicateReactions() + test.test_run() + From 73c465b83888d68ce50de357377bdec983e1866b Mon Sep 17 00:00:00 2001 From: carl-drews Date: Mon, 16 Sep 2024 08:54:32 -0600 Subject: [PATCH 04/11] Added the Pass test config. --- tests/integration/test_duplicate_reactions.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_duplicate_reactions.py b/tests/integration/test_duplicate_reactions.py index 7e54b039..2607dd1d 100644 --- a/tests/integration/test_duplicate_reactions.py +++ b/tests/integration/test_duplicate_reactions.py @@ -1,4 +1,5 @@ -from acom_music_box import MusicBox, Examples +from acom_music_box import MusicBox, Reaction, ReactionRate, Conditions +from acom_music_box.reaction_list import ReactionList import os import csv @@ -7,7 +8,16 @@ class TestDuplicateReactions: def test_run(self): - box_model = MusicBox() + # set up dummy reactions + abc123 = ReactionRate(Reaction("abc"), 12.3) + def456 = ReactionRate(Reaction("def"), 45.6) + abc789 = ReactionRate(Reaction("abc"), 78.9) + + # Pass: unique reaction names + pass_reactions = ReactionList(name="Pass list", reactions=[abc123, def456]) + pass_conditions = Conditions(reaction_rates=pass_reactions) + box_model = MusicBox(initial_conditions=pass_conditions) + box_model.check_config("Loaded from string.") assert True, f"All is good." From 69525bc76f09b8e7966ffcab690546a0f58f5fd4 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Mon, 16 Sep 2024 16:13:46 -0600 Subject: [PATCH 05/11] Finalized the test for duplicate reaction names. --- src/acom_music_box/main.py | 2 +- src/acom_music_box/music_box.py | 6 +++--- tests/integration/test_duplicate_reactions.py | 18 ++++++++++++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/acom_music_box/main.py b/src/acom_music_box/main.py index 782c9fba..075a206e 100644 --- a/src/acom_music_box/main.py +++ b/src/acom_music_box/main.py @@ -152,13 +152,13 @@ def main(): myBox = MusicBox() logger.debug(f"Configuration file = {musicBoxConfigFile}") myBox.readConditionsFromJson(musicBoxConfigFile) + myBox.check_config(os.path.join(os.getcwd(), musicBoxConfigFile)) # Create solver and solve config_path = os.path.join( os.path.dirname(musicBoxConfigFile), myBox.config_file) myBox.create_solver(config_path) - myBox.check_config(config_path) result = myBox.solve(musicBoxOutputPath) if musicBoxOutputPath is None: diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index 6d882ca1..e6fda5e3 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -91,7 +91,9 @@ def create_solver( def check_config(self, boxConfigPath): """ - Verifies correct configuration of the solver object. + Verifies correct configuration of the MusicBox object. + There is intentionally no check for the presence of a solver; + this test function is for the loaded configuration only. Args: boxConfigPath = filename and path of MusicBox configuration file @@ -102,8 +104,6 @@ def check_config(self, boxConfigPath): True if all checks passed Throws error for the first check failed. """ - if (not self.solver): - return(False) # look for duplicate reaction names if (self.initial_conditions): diff --git a/tests/integration/test_duplicate_reactions.py b/tests/integration/test_duplicate_reactions.py index 2607dd1d..46f38469 100644 --- a/tests/integration/test_duplicate_reactions.py +++ b/tests/integration/test_duplicate_reactions.py @@ -14,12 +14,26 @@ def test_run(self): abc789 = ReactionRate(Reaction("abc"), 78.9) # Pass: unique reaction names - pass_reactions = ReactionList(name="Pass list", reactions=[abc123, def456]) + pass_reactions = [abc123, def456] pass_conditions = Conditions(reaction_rates=pass_reactions) box_model = MusicBox(initial_conditions=pass_conditions) box_model.check_config("Loaded from string.") - assert True, f"All is good." + # Pass test should throw an error above if it fails + assert True, f"All is good." # example of assertion + + # Fail: duplicate reaction names + fail_reactions = reactions=[abc123, abc789] + fail_conditions = Conditions(reaction_rates=fail_reactions) + box_model = MusicBox(initial_conditions=fail_conditions) # new instance + + # catching the exception is a Pass for the duplicate case + caughtExcept = False + try: + box_model.check_config("Loaded from string.") + except Exception as myExcept: + caughtExcept = True + assert caughtExcept, f"Failed to detect duplicate reaction names." if __name__ == "__main__": From ca4358d50ece69be1e663626352d809cfc68aac0 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Fri, 20 Sep 2024 10:42:38 -0600 Subject: [PATCH 06/11] Moved from integration tests to unit tests. --- tests/{integration => unit}/test_duplicate_reactions.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{integration => unit}/test_duplicate_reactions.py (100%) diff --git a/tests/integration/test_duplicate_reactions.py b/tests/unit/test_duplicate_reactions.py similarity index 100% rename from tests/integration/test_duplicate_reactions.py rename to tests/unit/test_duplicate_reactions.py From 2a915054b90cc56ca28df3f6621ce329ddb662b7 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Fri, 20 Sep 2024 11:23:27 -0600 Subject: [PATCH 07/11] Use pytest to verify exception raised. --- tests/unit/test_duplicate_reactions.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_duplicate_reactions.py b/tests/unit/test_duplicate_reactions.py index 46f38469..f05df4e6 100644 --- a/tests/unit/test_duplicate_reactions.py +++ b/tests/unit/test_duplicate_reactions.py @@ -1,6 +1,7 @@ from acom_music_box import MusicBox, Reaction, ReactionRate, Conditions from acom_music_box.reaction_list import ReactionList +import pytest import os import csv import math @@ -27,13 +28,9 @@ def test_run(self): fail_conditions = Conditions(reaction_rates=fail_reactions) box_model = MusicBox(initial_conditions=fail_conditions) # new instance - # catching the exception is a Pass for the duplicate case - caughtExcept = False - try: + # verify that the fail exception was properly raised + with pytest.raises(Exception): box_model.check_config("Loaded from string.") - except Exception as myExcept: - caughtExcept = True - assert caughtExcept, f"Failed to detect duplicate reaction names." if __name__ == "__main__": From 22e5be662af69b7656bba0340afea7c1f7bfb11e Mon Sep 17 00:00:00 2001 From: carl-drews Date: Fri, 20 Sep 2024 11:28:33 -0600 Subject: [PATCH 08/11] Removed unneeded imports. --- tests/unit/test_duplicate_reactions.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/test_duplicate_reactions.py b/tests/unit/test_duplicate_reactions.py index f05df4e6..93c9751b 100644 --- a/tests/unit/test_duplicate_reactions.py +++ b/tests/unit/test_duplicate_reactions.py @@ -2,9 +2,6 @@ from acom_music_box.reaction_list import ReactionList import pytest -import os -import csv -import math class TestDuplicateReactions: From 6b8bc647bc0115f84d52c31e381d8085d7ae6fb2 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Fri, 20 Sep 2024 11:40:38 -0600 Subject: [PATCH 09/11] Simpler assert check for success. --- tests/unit/test_duplicate_reactions.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/unit/test_duplicate_reactions.py b/tests/unit/test_duplicate_reactions.py index 93c9751b..d5262e69 100644 --- a/tests/unit/test_duplicate_reactions.py +++ b/tests/unit/test_duplicate_reactions.py @@ -15,10 +15,7 @@ def test_run(self): pass_reactions = [abc123, def456] pass_conditions = Conditions(reaction_rates=pass_reactions) box_model = MusicBox(initial_conditions=pass_conditions) - box_model.check_config("Loaded from string.") - - # Pass test should throw an error above if it fails - assert True, f"All is good." # example of assertion + assert box_model.check_config("Loaded from string.") # Fail: duplicate reaction names fail_reactions = reactions=[abc123, abc789] From bbf551c129b19cd8a82a062e5426b3d4cbcadf30 Mon Sep 17 00:00:00 2001 From: carl-drews Date: Fri, 20 Sep 2024 11:58:47 -0600 Subject: [PATCH 10/11] Moved the configuration check to within readConditionsFromJson(), and at the end. --- src/acom_music_box/main.py | 1 - src/acom_music_box/music_box.py | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/acom_music_box/main.py b/src/acom_music_box/main.py index 075a206e..9e1c7f8e 100644 --- a/src/acom_music_box/main.py +++ b/src/acom_music_box/main.py @@ -152,7 +152,6 @@ def main(): myBox = MusicBox() logger.debug(f"Configuration file = {musicBoxConfigFile}") myBox.readConditionsFromJson(musicBoxConfigFile) - myBox.check_config(os.path.join(os.getcwd(), musicBoxConfigFile)) # Create solver and solve config_path = os.path.join( diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index e6fda5e3..0caba33a 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -321,6 +321,9 @@ def readConditionsFromJson(self, path_to_json): self.evolving_conditions = EvolvingConditions.from_config_JSON( path_to_json, data, self.species_list, self.reaction_list) + self.check_config(os.path.join(os.getcwd(), path_to_json)) + + def speciesOrdering(self): """ Retrieves the ordering of species used in the solver. From 9ff025d483697b0e748100b6afcc8d37228b594f Mon Sep 17 00:00:00 2001 From: carl-drews Date: Tue, 24 Sep 2024 11:11:49 -0600 Subject: [PATCH 11/11] Fixed indentation mistake. --- src/acom_music_box/music_box.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/acom_music_box/music_box.py b/src/acom_music_box/music_box.py index fe3b6ac7..50583fa5 100644 --- a/src/acom_music_box/music_box.py +++ b/src/acom_music_box/music_box.py @@ -69,11 +69,11 @@ def add_evolving_condition(self, time_point, conditions): time=[time_point], conditions=[conditions]) self.evolvingConditions.append(evolving_condition) - def create_solver( - self, - path_to_config, - solver_type=musica.micmsolver.rosenbrock, - number_of_grid_cells=1): + def create_solver( + self, + path_to_config, + solver_type=musica.micmsolver.rosenbrock, + number_of_grid_cells=1): """ Creates a micm solver object using the CAMP configuration files.