Skip to content

Conversation

@iraedeus
Copy link
Collaborator

No description provided.

@iraedeus iraedeus requested a review from Copilot July 17, 2025 15:48
Copy link

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

This PR aims to improve determinism across the experimental environment by standardizing seed usage, updating estimator naming, enhancing the error summarizer, and adding an end-to-end reproducibility test.

  • Consolidate and propagate RNG seeding in dataset generators and executors for reproducible experiments
  • Rename LMomentsEstimator.name from "LM-EM" to "ELM" and adjust related tests
  • Refactor error summarizer to use NumPy functions and handle empty-result cases

Reviewed Changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/experimental_env/test_reproducibility_expenv.py Added full reproducibility scenario test
tests/core/test_problem.py Added Hypothesis tests for Problem
tests/core/test_mixture_distribution.py Extended unit and integration tests for mixture distribution
tests/core/test_distribution.py Extended unit and integration tests for distribution
experimental_env/preparation/dataset_generator.py Moved RNG seeding into base RandomDatasetGenerator
experimental_env/mixture_generators/utils.py Tweaked default Weibull parameters
experimental_env/mixture_generators/abstract_generator.py Removed default seeding from abstract generator
experimental_env/experiment/experiment_executors/standart_executor.py Dropped seed forwarding to standard mixture generator
experimental_env/experiment/experiment_executors/random_executor.py Dropped seed forwarding to random mixture generator
experimental_env/experiment/experiment_executors/abstract_executor.py Added seed-based RNG initialization but missing NumPy import
experimental_env/experiment/estimators.py Renamed L-moments estimator identifier from "LM-EM" to "ELM"
experimental_env/analysis/analyze_summarizers/error_summarizer.py Switched to NumPy for stats and added empty-list guard
Comments suppressed due to low confidence (3)

experimental_env/preparation/dataset_generator.py:63

  • ConcreteDatasetGenerator no longer seeds the RNG, breaking deterministic behavior. Re-add random.seed(self._seed) and np.random.seed(self._seed) inside its __init__ to maintain reproducibility.
    def __init__(self, seed: int = 42):

tests/core/test_problem.py:48

  • [nitpick] The docstring is in Russian, which may be unclear to other maintainers. Consider translating it to English for consistency.
        """Тест инициализации Problem."""

experimental_env/experiment/experiment_executors/abstract_executor.py:37

  • The module references np.random but does not import NumPy. Add import numpy as np at the top to avoid a NameError.
        np.random.seed(self._seed)

@iraedeus iraedeus merged commit d8b9c5f into PySATL:main Jul 17, 2025
3 checks passed
@iraedeus iraedeus deleted the iraedeus/fix-expenv branch July 17, 2025 16:07
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.

1 participant