Skip to content

fixed bug where new runs would always be 50 percent, and added a headless execution option #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
giuseppe-zappia wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fixed bug where new runs would always be 50 percent, and added a headless execution option #20

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b376ae
fixed bug where new runs would always be 50 percent
giuseppe-zappia Oct 13, 2025
1996495
Added arguments for headless execution
giuseppe-zappia Oct 14, 2025
62683af
Update spectrum.py
giuseppe-zappia Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions spectrum.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ def main():
parser = argparse.ArgumentParser(description="Process SNR data for layers.")
parser.add_argument('--model-name', type=str, required=True, help='Model name or path to the model')
parser.add_argument('--top-percent', type=int, default=None, help='Top percentage of layers to select, overriding the default')
parser.add_argument('--all-layer-types', type=bool, default=False, help='Whether to include all layers in selection')
parser.add_argument('--batch-size', type=int, default=None, help='Batch size to use in SnR calculation')
args = parser.parse_args()

# Check for existing SNR results file
Expand All @@ -244,12 +246,29 @@ def main():
print(f"Found existing SNR results file for {args.model_name}")
modifier = ModelModifier(top_percent=args.top_percent)
modifier.generate_unfrozen_params_yaml(snr_file_path, args.top_percent)

weight_types = modifier.get_weight_types()
print(weight_types)
Comment on lines +249 to +251
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: AttributeError when model is None.

When an existing SNR file is found, ModelModifier is instantiated without model_name (line 247), setting self.model = None (line 49). Calling modifier.get_weight_types() on line 250 then fails because it tries to iterate over self.model.named_modules() (line 56) when self.model is None.

Apply this diff to pass the model name when an existing SNR file exists:

     if os.path.exists(snr_file_path):
         print(f"Found existing SNR results file for {args.model_name}")
-        modifier = ModelModifier(top_percent=args.top_percent)
+        modifier = ModelModifier(model_name=args.model_name, top_percent=args.top_percent)
         modifier.generate_unfrozen_params_yaml(snr_file_path, args.top_percent)
 
         weight_types = modifier.get_weight_types()
         print(weight_types)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
weight_types = modifier.get_weight_types()
print(weight_types)
if os.path.exists(snr_file_path):
print(f"Found existing SNR results file for {args.model_name}")
modifier = ModelModifier(model_name=args.model_name, top_percent=args.top_percent)
modifier.generate_unfrozen_params_yaml(snr_file_path, args.top_percent)
weight_types = modifier.get_weight_types()
print(weight_types)
🤖 Prompt for AI Agents 
spectrum.py around lines 249 to 251: when an existing SNR file is found the code
instantiates ModelModifier without a model name so self.model is None and
calling modifier.get_weight_types() raises AttributeError; fix this by passing
the current model_name (or the variable that holds the model identifier) into
the ModelModifier constructor where the existing SNR file branch creates the
modifier so that ModelModifier initializes with a valid model instance before
get_weight_types() is called.

else:
print(f"No existing SNR results file found for {args.model_name}. Proceeding with SNR calculation.")
batch_size = input_dialog(title="Batch Size", text="Enter the batch size:").run()

if args.batch_size is None:
batch_size = input_dialog(title="Batch Size", text="Enter the batch size:").run()
else:
batch_size = args.batch_size

batch_size = int(batch_size) if batch_size else 1
modifier = ModelModifier(model_name=args.model_name, batch_size=batch_size)
selected_weight_types = modifier.interactive_select_weights()

modifier = ModelModifier(model_name=args.model_name, batch_size=batch_size, top_percent=args.top_percent)
Copy link

@coderabbitai coderabbitai bot Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Explicitly passing None overrides the default and causes TypeError.

When the user doesn't provide --top-percent, args.top_percent is None. Passing top_percent=None explicitly to ModelModifier overrides the default parameter value of 50 in the constructor (line 13), setting self.top_percent = None. This None value propagates through save_snr_to_json() → generate_unfrozen_params_yaml() and causes a TypeError at line 197: int(len(layers) * None / 100).

Apply this diff to only pass top_percent when it's not None:

-        modifier = ModelModifier(model_name=args.model_name, batch_size=batch_size, top_percent=args.top_percent)
+        modifier = ModelModifier(
+            model_name=args.model_name,
+            batch_size=batch_size,
+            top_percent=args.top_percent if args.top_percent is not None else 50
+        )

Alternatively, if you adopt the refactor suggested in the previous comment (setting the argparse default to 50), the current code would work correctly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
modifier = ModelModifier(model_name=args.model_name, batch_size=batch_size, top_percent=args.top_percent)
modifier = ModelModifier(
model_name=args.model_name,
batch_size=batch_size,
top_percent=args.top_percent if args.top_percent is not None else 50
)
🤖 Prompt for AI Agents 
In spectrum.py around line 251, passing args.top_percent (which may be None)
into ModelModifier overrides the constructor default of 50 and later causes a
TypeError when used in arithmetic; fix by only including top_percent in the
ModelModifier call when args.top_percent is not None (e.g., construct with
ModelModifier(model_name=..., batch_size=batch_size) and only add
top_percent=args.top_percent when args.top_percent is not None), or
alternatively set the argparse default for --top-percent to 50 so
args.top_percent is never None.


if args.all_layer_types:
weight_types = modifier.get_weight_types()
print(f"selecting all weight types: {weight_types}")
selected_weight_types = modifier.sort_weight_types(weight_types)
modifier.layer_types = selected_weight_types
else:
selected_weight_types = modifier.interactive_select_weights()

if selected_weight_types:
modifier.assess_layers_snr(selected_weight_types)
modifier.save_snr_to_json()
Expand All @@ -258,4 +277,4 @@ def main():
print("No weight types selected.")

if __name__ == "__main__":
main()
main()