From 0669a455cef5c7449b4cc3ae187b25883cb6ada8 Mon Sep 17 00:00:00 2001 From: Mansib Rahman Date: Sun, 14 Jun 2026 17:11:02 -0700 Subject: [PATCH 01/11] created CONTRIBUTING.md file --- CONTRIBUTING.md | 17 +++++++++++++++++ toolchain/mfc/case_validator.py | 3 ++- toolchain/mfc/params/ast_analyzer.py | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..2666d39a3f --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,17 @@ +Part 1: Reproducing the Issue + +To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead like so: + +# chemistry = self.get("chemistry", "F") == "T" +chemistry = self.flag("chemistry") + +The terminal blanks out seen in the following output: +chemistry (`chemistry`) + +I expect the computer to know what to do when it is given a new function like flag() and output the correct result. + +Part 2: Solution Plan + +1. The root cause is the hard coding in ast_analyzer.py in the _build_local_param_map() function in line 232: call.func.attr == "get". Thus, when the analyzer sees the word "flag", it doesn't recognize it and blanks out. + +2. My solution is to implement a flag() method that maps calls the get() method \ No newline at end of file diff --git a/toolchain/mfc/case_validator.py b/toolchain/mfc/case_validator.py index 4039cc94ad..758d0337b8 100644 --- a/toolchain/mfc/case_validator.py +++ b/toolchain/mfc/case_validator.py @@ -1484,7 +1484,8 @@ def check_chemistry(self): runtime if misconfigured. """ # Fetch global chemistry and diffusion flags - chemistry = self.get("chemistry", "F") == "T" + # chemistry = self.get("chemistry", "F") == "T" + chemistry = self.flag("chemistry") diffusion = self.get("chem_params%diffusion", "F") == "T" num_fluids = self.get("num_fluids") diff --git a/toolchain/mfc/params/ast_analyzer.py b/toolchain/mfc/params/ast_analyzer.py index ad7d94fd42..9a1b8acbf5 100644 --- a/toolchain/mfc/params/ast_analyzer.py +++ b/toolchain/mfc/params/ast_analyzer.py @@ -235,6 +235,7 @@ def _build_local_param_map(self, func: ast.FunctionDef) -> Dict[str, str]: and isinstance(call.args[0].value, str) ): param_name = call.args[0].value + print(f"Issue reproduced: {param_name}") for target in node.targets: if isinstance(target, ast.Name): m[target.id] = param_name From b044485b2e7e20fa9f72ef24c6eb4765caa07d4e Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Sun, 14 Jun 2026 17:29:24 -0700 Subject: [PATCH 02/11] Revised CONTRIBUTING.md --- CONTRIBUTING.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2666d39a3f..1ee8d49f4f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,8 +1,7 @@ -Part 1: Reproducing the Issue +# Reproduction Process To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead like so: -# chemistry = self.get("chemistry", "F") == "T" chemistry = self.flag("chemistry") The terminal blanks out seen in the following output: @@ -10,8 +9,14 @@ The terminal blanks out seen in the following output: I expect the computer to know what to do when it is given a new function like flag() and output the correct result. -Part 2: Solution Plan +# Solution Approach -1. The root cause is the hard coding in ast_analyzer.py in the _build_local_param_map() function in line 232: call.func.attr == "get". Thus, when the analyzer sees the word "flag", it doesn't recognize it and blanks out. +1. At the moment, case_validator.py reads boolean case flags with the verbose form self.get("name", "F") == "T" in 134 places. To clean this code up I want to implement a map function. However, I am prevented from doing so at the moment because of the hard coding in ast_analyzer.py in the _build_local_param_map() method in line 232: call.func.attr == "get". Thus, when the analyzer sees the word "flag", it doesn't recognize it and blanks out. -2. My solution is to implement a flag() method that maps calls the get() method \ No newline at end of file +2. My solution is to first make sure the analyzer recognizes flag() by adding a conditional statement in _build_local_param_map() in ast_analyzer.py. I will then implement a flag() method in case_validator.py that maps the verbose form to a simple boolean value. + +3. Files I will touch include: + - case_validator.py + - ast_analyzer.py + +4. I will verify my approach works by calling flag() instead of get() in line 1487 of case_validator.py. If the analyzer correctly returns a value instead of blank, it successfully recognized the flag() function. I will also test the flag() method itself to ensure it correctly returns true or false. From 15d43a6ec907a88be3a8cce06ab8938a3579fbd0 Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Sun, 14 Jun 2026 17:43:45 -0700 Subject: [PATCH 03/11] Added issue reproduction procedure and branch link to CONTRIBUTING.md --- CONTRIBUTING.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1ee8d49f4f..b312029596 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,13 +1,21 @@ # Reproduction Process -To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead like so: - -chemistry = self.flag("chemistry") +To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead like so: chemistry = self.flag("chemistry") The terminal blanks out seen in the following output: chemistry (`chemistry`) -I expect the computer to know what to do when it is given a new function like flag() and output the correct result. +I expect the computer to know what to do when it is given a function like flag() and output the correct result. + +Environment Setup: I did not face any problems during the environment setup process as I have been using. + +Steps to Reproduce: + 1. Open the file case_validator.py using the file path toolchain/mfc/case_validator.py + 2. Go to line 1487 in the check_chemistry() method + 3. Replace the value assigned to the chemistry variable with self.flag("chemistry") + 4. Reproduce the issue on your terminal by running the following: python3 toolchain/mfc/gen_case_constraints_docs.py | grep "ANALYZER" + +Branch Link: https://github.com/mansibrahman03/MFlowCode/tree/main # Solution Approach From 44052792f65d0fb5f885ad07a7ec107c1597c9a2 Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Sun, 14 Jun 2026 17:45:21 -0700 Subject: [PATCH 04/11] Fixed formating in CONTRIBUTING.md --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b312029596..25454d5203 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ # Reproduction Process -To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead like so: chemistry = self.flag("chemistry") +To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead. The terminal blanks out seen in the following output: chemistry (`chemistry`) @@ -19,6 +19,8 @@ Branch Link: https://github.com/mansibrahman03/MFlowCode/tree/main # Solution Approach +Implementation: + 1. At the moment, case_validator.py reads boolean case flags with the verbose form self.get("name", "F") == "T" in 134 places. To clean this code up I want to implement a map function. However, I am prevented from doing so at the moment because of the hard coding in ast_analyzer.py in the _build_local_param_map() method in line 232: call.func.attr == "get". Thus, when the analyzer sees the word "flag", it doesn't recognize it and blanks out. 2. My solution is to first make sure the analyzer recognizes flag() by adding a conditional statement in _build_local_param_map() in ast_analyzer.py. I will then implement a flag() method in case_validator.py that maps the verbose form to a simple boolean value. From 3095f7e82fee2d8ffdb96b89018870a40617eb85 Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Sun, 14 Jun 2026 17:46:41 -0700 Subject: [PATCH 05/11] Added observations in CONTRIBUTING.md --- CONTRIBUTING.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 25454d5203..f06b5874ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,11 +1,9 @@ # Reproduction Process -To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead. - -The terminal blanks out seen in the following output: -chemistry (`chemistry`) - -I expect the computer to know what to do when it is given a function like flag() and output the correct result. +Observations: + - To repoduce the issue, I first commented out the call to get() in line 1487 and replaced it with a call to flag instead. + - The terminal blanks and doesn't output anything when the flag() method is called. + - I expect the computer to know what to do when it is given a function like flag() and output the correct result. Environment Setup: I did not face any problems during the environment setup process as I have been using. From 36b774f137507c74c75156313c702c0e8cbbe638 Mon Sep 17 00:00:00 2001 From: Mansib Rahman Date: Tue, 23 Jun 2026 13:43:04 -0700 Subject: [PATCH 06/11] edited build_local_param_map() to recognize flag function --- toolchain/mfc/params/ast_analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolchain/mfc/params/ast_analyzer.py b/toolchain/mfc/params/ast_analyzer.py index 9a1b8acbf5..e58e001579 100644 --- a/toolchain/mfc/params/ast_analyzer.py +++ b/toolchain/mfc/params/ast_analyzer.py @@ -229,7 +229,7 @@ def _build_local_param_map(self, func: ast.FunctionDef) -> Dict[str, str]: isinstance(call.func, ast.Attribute) and isinstance(call.func.value, ast.Name) and call.func.value.id == "self" - and call.func.attr == "get" + and call.func.attr in ("get", "flag") and call.args and isinstance(call.args[0], ast.Constant) and isinstance(call.args[0].value, str) From bc4f96e0a758486c7709b1fdb3f6b3e678dd7f5b Mon Sep 17 00:00:00 2001 From: Mansib Rahman Date: Tue, 23 Jun 2026 13:50:32 -0700 Subject: [PATCH 07/11] Added flag() function to case_validator.py --- toolchain/mfc/case_validator.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/toolchain/mfc/case_validator.py b/toolchain/mfc/case_validator.py index 758d0337b8..4a998ae8ab 100644 --- a/toolchain/mfc/case_validator.py +++ b/toolchain/mfc/case_validator.py @@ -246,6 +246,10 @@ def __init__(self, params: Dict[str, Any]): def get(self, key: str, default=None): """Get parameter value with default""" return self.params.get(key, default) + + def flag(self, name: str) -> bool: + """True if and only if the case sets `name` to "T".""" + return self.get(name, "F") == "T" def is_set(self, key: str) -> bool: """Check if parameter is set (not None and present)""" From 4e1b5fa4ce7a3d7052d483b36006d3ee01d4a7f6 Mon Sep 17 00:00:00 2001 From: Mansib Rahman Date: Tue, 23 Jun 2026 14:35:15 -0700 Subject: [PATCH 08/11] Added tests for CaseValidator.flag() and ast_analyzer._build_local_param_map fix --- .../mfc/params_tests/test_flag_helper.py | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 toolchain/mfc/params_tests/test_flag_helper.py diff --git a/toolchain/mfc/params_tests/test_flag_helper.py b/toolchain/mfc/params_tests/test_flag_helper.py new file mode 100644 index 0000000000..a685481cd8 --- /dev/null +++ b/toolchain/mfc/params_tests/test_flag_helper.py @@ -0,0 +1,76 @@ +""" +Tests for CaseValidator.flag() and ast_analyzer._build_local_param_map fix. +""" + +import ast +import unittest + +from ..case_validator import CaseValidator +from ..params.ast_analyzer import CaseValidatorAnalyzer + + +def _param_map(source: str) -> dict: + """Parse a single method body and return its local-param map.""" + wrapped = "class _V:\n" + "\n".join(" " + l for l in source.splitlines()) + func = ast.parse(wrapped).body[0].body[0] + a = CaseValidatorAnalyzer.__new__(CaseValidatorAnalyzer) + return a._build_local_param_map(func) + + +class TestFlag(unittest.TestCase): + + def test_true_when_set_to_T(self): + self.assertTrue(CaseValidator({"chemistry": "T"}).flag("chemistry")) + + def test_false_when_set_to_F(self): + self.assertFalse(CaseValidator({"chemistry": "F"}).flag("chemistry")) + + def test_false_when_absent(self): + self.assertFalse(CaseValidator({}).flag("chemistry")) + + def test_only_uppercase_T_is_truthy(self): + """Lowercase, integers, and booleans must not count as set.""" + for val in ("t", "True", 1, True, None): + self.assertFalse(CaseValidator({"x": val}).flag("x"), f"Expected False for {val!r}") + + def test_returns_bool(self): + self.assertIsInstance(CaseValidator({"x": "T"}).flag("x"), bool) + + +class TestASTCoupling(unittest.TestCase): + + def test_flag_call_recorded_by_analyzer(self): + """self.flag('x') must register 'x' in the param map, same as self.get('x','F')=='T'.""" + m = _param_map("def check(self):\n chemistry = self.flag('chemistry')") + self.assertEqual(m.get("chemistry"), "chemistry") + + def test_get_call_still_recorded(self): + """Existing self.get() pattern must not regress.""" + m = _param_map("def check(self):\n d = self.get('chem_params%diffusion', 'F') == 'T'") + self.assertEqual(m.get("d"), "chem_params%diffusion") + + def test_old_matcher_would_have_missed_flag(self): + """Demonstrate the pre-fix bug: attr=='get' silently dropped flag() calls.""" + source = "def check(self):\n chemistry = self.flag('chemistry')" + wrapped = "class _V:\n" + "\n".join(" " + l for l in source.splitlines()) + func = ast.parse(wrapped).body[0].body[0] + + broken = {} + for node in ast.walk(func): + if isinstance(node, ast.Assign): + v = node.value.left if isinstance(node.value, ast.Compare) else node.value + if isinstance(v, ast.Call) and isinstance(v.func, ast.Attribute): + if v.func.value.id == "self" and v.func.attr == "get": # old check + for t in node.targets: + if isinstance(t, ast.Name): + broken[t.id] = v.args[0].value + + self.assertNotIn("chemistry", broken, "Old matcher should miss flag() — that was the bug") + + a = CaseValidatorAnalyzer.__new__(CaseValidatorAnalyzer) + fixed = a._build_local_param_map(func) + self.assertIn("chemistry", fixed, "Fixed matcher must capture flag() calls") + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From b989646dc076b7be2df05f56131ac8af804c0ded Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Tue, 23 Jun 2026 14:46:44 -0700 Subject: [PATCH 09/11] Documented implementation progress Updated implementation notes and testing strategy in CONTRIBUTING.md. --- CONTRIBUTING.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f06b5874ed..fcce7e283f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,3 +28,13 @@ Implementation: - ast_analyzer.py 4. I will verify my approach works by calling flag() instead of get() in line 1487 of case_validator.py. If the analyzer correctly returns a value instead of blank, it successfully recognized the flag() function. I will also test the flag() method itself to ensure it correctly returns true or false. + +# Implementation Notes + +Implementation Progress: I fixed _build_local_param_map() in ast_analyzer.py to recognize flag() method and added a flag() helper method in case_validator.py. + +Challenges Faced: The main challenge was understanding the codebase. + +Testing Strategy: I tested the flag method itself to ensure it correctly return a bool for all possible params (T, F, null). I also test to see that the old broken code fails and the new code works. + +Branch Link: https://github.com/mansibrahman03/MFlowCode/tree/main From 4cd78c2036ae34741b80439b128383efd1279de5 Mon Sep 17 00:00:00 2001 From: Mansib Rahman Date: Tue, 30 Jun 2026 16:19:57 -0700 Subject: [PATCH 10/11] cleaned up commented code --- toolchain/mfc/case_validator.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/toolchain/mfc/case_validator.py b/toolchain/mfc/case_validator.py index 4a998ae8ab..a134ba9cab 100644 --- a/toolchain/mfc/case_validator.py +++ b/toolchain/mfc/case_validator.py @@ -1487,8 +1487,6 @@ def check_chemistry(self): is provided. No static validation is performed here - chemistry will fail at runtime if misconfigured. """ - # Fetch global chemistry and diffusion flags - # chemistry = self.get("chemistry", "F") == "T" chemistry = self.flag("chemistry") diffusion = self.get("chem_params%diffusion", "F") == "T" num_fluids = self.get("num_fluids") From c188d3707f1834539705e7609232e5599e11e061 Mon Sep 17 00:00:00 2001 From: mansibrahman03 Date: Tue, 30 Jun 2026 17:15:32 -0700 Subject: [PATCH 11/11] Added PR notes --- CONTRIBUTING.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fcce7e283f..63356c599c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,3 +38,13 @@ Challenges Faced: The main challenge was understanding the codebase. Testing Strategy: I tested the flag method itself to ensure it correctly return a bool for all possible params (T, F, null). I also test to see that the old broken code fails and the new code works. Branch Link: https://github.com/mansibrahman03/MFlowCode/tree/main + +# PR + +PR Link: [Direct link to your submitted pull request](https://github.com/MFlowCode/MFC/pull/1622) + +PR Description: I added a flag() method in toolchain/mfc/case_validator.py that functions to replace the verbose idiom "self.get(x,F)==T" repeated in 134 places across the file. I also updated ast_analyzer.py to recognize calls made to flag(). + +Maintainer Feedback: I am currently waiting for maintainer to respond. + +Status: Awaiting review