Skip to content

Update BenchmarkData to be consistent with results system_group system_name terminology - #137

Merged
jaclark5 merged 62 commits into
mainfrom
benchmarkdata
Sep 4, 2026
Merged

Update BenchmarkData to be consistent with results system_group system_name terminology#137
jaclark5 merged 62 commits into
mainfrom
benchmarkdata

Conversation

@jaclark5

Copy link
Copy Markdown
Collaborator

Unify terminology between BenchmarkData and results/annotations API, which already use system_group and system_name. This standardizes naming conventions across the codebase.

Core terminology mappings applied throughout the codebase:

Old Term New Term Context
benchmark_set system_group Parameter, attribute, and variable names
BenchmarkData.name BenchmarkData.system_name Dataclass field
list_benchmark_sets() list_system_groups() BenchmarkIndex method
list_systems_by_benchmark_set() list_system_names_by_group() BenchmarkIndex method
list_systems_by_tag() list_system_names_by_tag() BenchmarkIndex method
get_benchmark_data_system() get_data_by_system_name() Factory function
get_benchmark_set_data_systems() get_data_by_system_group() Factory function
benchmark_sets_systems system_groups_systems Dataclass field (AutoMetadata)
benchmark_system system_name SystemInfo dataclass field
"benchmark set" "system group" Documentation and comments

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request standardizes naming across the benchmarks codebase to align BenchmarkData and associated indexing/factory APIs with the results/annotations terminology (system_group, system_name). It also updates results utilities and documentation/examples to match these public API changes (including FEMap access patterns and filtering helpers).

Changes:

  • Renames BenchmarkData- and index-related APIs from “benchmark_set/benchmark_system” to “system_group/system_name” across library code, tests, scripts, and notebooks.
  • Refactors results filtering into a dedicated module and updates BenchmarkResults FEMap access to callable methods (dg_femaps(), ddg_femaps()) with optional source labeling.
  • Adds docstring validation tests and updates example scripts/notebooks to use the new public APIs.

Reviewed changes

Copilot reviewed 20 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
openfe_benchmarks/tests/test_results_validation.py Updates FEMap access to method calls (ddg_femaps(), dg_femaps()).
openfe_benchmarks/tests/test_docstrings.py Adds docstring/doctest validation coverage for results/filtering modules.
openfe_benchmarks/tests/test_benchmark_results.py Adjusts tests to new FEMap access and filtering behavior (now raising on invalid version comparisons).
openfe_benchmarks/tests/test_benchmark_index.py Renames index API usages to list_system_groups() / list_system_names_by_tag().
openfe_benchmarks/tests/test_benchmark_data.py Updates BenchmarkData field names and data access factory function names.
openfe_benchmarks/scripts/prepare_metadata_submission.py Renames metadata structures/fields to system_group/system_name and updates YAML rendering helpers.
openfe_benchmarks/scripts/_no_test_example_plot_asfe.py Updates example to use dg_femaps() method.
openfe_benchmarks/scripts/_example_plot_rbfe.py Updates example to use ddg_femaps() method.
openfe_benchmarks/scripts/_example_plan_septop.py Updates planning example constants and data access to new naming (get_data_by_system_name).
openfe_benchmarks/scripts/_example_plan_rbfe.py Updates planning example constants and data access to new naming.
openfe_benchmarks/scripts/_example_plan_rbfe_pontibus.py Updates planning example constants and data access to new naming.
openfe_benchmarks/scripts/_example_plan_asfe.py Updates planning example constants and data access to new naming.
openfe_benchmarks/results/2026_08_25_ff14sb_openff-2.1.1_jacs/submission.yaml Adds/updates a submission metadata file (terminology needs alignment with system_group wording).
openfe_benchmarks/results/_results_utils.py Updates BenchmarkData fetch function and adds configurable source for cinnabar FEMap measurements.
openfe_benchmarks/results/_filtering.py Introduces a standalone filtering implementation used by filter_results.
openfe_benchmarks/results/_benchmark_results.py Integrates new filtering module, expands docs/examples, and converts FEMap accessors to methods with source.
openfe_benchmarks/data/README.md Updates directory terminology from benchmark_set to system_group.
openfe_benchmarks/data/_benchmark_systems.py Renames public APIs and BenchmarkData fields to system_group/system_name.
openfe_benchmarks/data/init.py Re-exports renamed factory functions.
examples/2_benchmark_data_with_openfe.ipynb Updates notebook imports and output references to new BenchmarkData fields and factory names.
examples/1_initializing_benchmark_data.ipynb Updates notebook narrative/code to new system_group/system_name terminology and API names.
Suppressed comments (2)

openfe_benchmarks/data/_benchmark_systems.py:82

  • This method uses a mutable default (tags: list[str] = []) and the docstring examples still reference the old API (get_systems_by_tag). Using an immutable default avoids the common Python pitfall, and updating the examples keeps the docstring accurate.
    def list_system_names_by_tag(self, tags: list[str] = []) -> list[tuple[str, str]]:
        """
        Get all systems that match **all** of the provided tags.

        Parameters

openfe_benchmarks/scripts/prepare_metadata_submission.py:1030

  • This error message still uses the old "set/system" terminology. Since this PR standardizes on system_group/system_name, update the wording here for consistency.
                raise ValueError(
                    f"Transformation detects a count other than one ligand: network_key={metadata.network_key}, set/system: {system_group_n_name}, transformation: {trans.name}, ligands: {system_info['ligands']}"
                )

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread openfe_benchmarks/tests/test_docstrings.py Outdated
Comment thread openfe_benchmarks/tests/test_docstrings.py
Comment thread openfe_benchmarks/tests/test_docstrings.py
Comment thread openfe_benchmarks/scripts/prepare_metadata_submission.py
Base automatically changed from benchmark_results to main September 4, 2026 19:27
@jaclark5 jaclark5 self-assigned this Sep 4, 2026
@jaclark5
jaclark5 requested a balanced review from Copilot September 4, 2026 19:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Three committed submission.yaml summaries were hand-edited to the incorrect term "system names" (should be "system group"), which diverges from the still-unchanged generator wording and from four other unedited result files.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 17/18 changed files
  • Comments generated: 4
  • Review effort level: Balanced

def _generate_title(
mode: str,
benchmark_set_systems: list[tuple[str, str]],
system_names: list[tuple[str, str]],
@jaclark5
jaclark5 merged commit df4709a into main Sep 4, 2026
6 checks passed
@jaclark5
jaclark5 deleted the benchmarkdata branch September 4, 2026 19:59
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