Skip to content

[Issue 528] Make cluster point process code more general by fixing hard coded values#937

Open
CodersRepo wants to merge 4 commits into
SharedDevelopmentfrom
issue-528-fixing-hard-coded-cluster-point-process-code
Open

[Issue 528] Make cluster point process code more general by fixing hard coded values#937
CodersRepo wants to merge 4 commits into
SharedDevelopmentfrom
issue-528-fixing-hard-coded-cluster-point-process-code

Conversation

@CodersRepo
Copy link
Copy Markdown

@CodersRepo CodersRepo commented May 14, 2026

…ph path

Closes #528

Description

Cluster point process: configurable params, JSON CLI, fix ignored graph path

Use the graph file chosen in the UI instead of a hardcoded spd.graphml path.
Add generate_cluster_point_process_xml() for shared GUI/CLI generation with
optional RNG seed, prototype selection weights, and XML clock attributes.

Extend secprocess() with optional prototype_weights; keep legacy 40/50/9/1
weights for four prototypes when weights are omitted; factor Tukey fence
constant. Add --config for headless runs from JSON; validate graph path and
fix mismatched error labels.

Checklist (Mandatory for new features)

  • Added Documentation
  • Added Unit Tests

Testing (Mandatory for all changes)

  • GPU Test: test-medium-connected.xml Passed
  • GPU Test: test-large-long.xml Passed

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process_functions.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
CodersRepo and others added 2 commits May 18, 2026 13:19
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py:626

  • The GUI validation currently attempts to cast every non-seed field to float (including "Graph ID:"). GraphML node IDs are strings and may be non-numeric; even numeric IDs can be entered as "194.0"/"1e2" which passes float() but won’t match the actual node key, leading to a confusing KeyError later. Treat Graph ID as a string (or validate it as an integer-like string) and defer existence checking to the graph lookup.
                else:
                    try:
                        float(text)  # Attempt to convert to float to check validity
                    except ValueError:

Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py:650

  • The warning text says "(float values only)", but this form now includes an integer-only seed field and (ideally) a string graph id. This message is now misleading; update it to reflect per-field expectations (e.g., numeric fields vs optional integer seed vs string graph id).
        if invalid_fields:
            error_message = "Invalid or empty values in the following fields: (float values only)\n"
            for field in invalid_fields:
                error_message += f"- {field}\n"

Comment thread Tools/InputGeneration/ClusterPointProcess/cluster_point_process.py
Comment on lines +126 to +130
if prototype_weights is None:
if n == len(DEFAULT_LEGACY_PROTOTYPE_WEIGHTS):
w = np.array(DEFAULT_LEGACY_PROTOTYPE_WEIGHTS, dtype=float)
else:
w = np.ones(n, dtype=float) / n
@CodersRepo CodersRepo requested a review from stiber May 20, 2026 21:19
@stiber
Copy link
Copy Markdown
Contributor

stiber commented May 22, 2026

There are a lot of changes here, and while I could just approve it, I think we need to take advantage of this as a opportunity to add in testing so we can be confident that it produces good output. Here are my suggestions:

  • First of all, maybe we can go over this together so I'm convinced it works properly.
  • Then, from that experience, let's figure out what kind of testing would make sense in the future. Presumably, this would be something like unit and integration tests, but maybe just a set of tests without distinction.
  • Then, let's create an issue for generating a new GitHub action to perform tests of the Tools whenever any file in there is modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants