Skip to content

[#144] Resource reward randomization#278

Merged
Mark2000 merged 1 commit into
developfrom
feature/reward-randomization
Jun 12, 2025
Merged

[#144] Resource reward randomization#278
Mark2000 merged 1 commit into
developfrom
feature/reward-randomization

Conversation

@Mark2000

Copy link
Copy Markdown
Contributor

Description

Allows for the resource reward weight to be randomized per-episode for multi objective cases.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How should this pull request be reviewed?

  • By commit
  • All changes at once

How Has This Been Tested?

Added tests.

Future Work

Generalize to all rewarders.

Checklist

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation and release notes
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Copilot AI review requested due to automatic review settings June 12, 2025 16:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for per-episode randomization of resource reward weights and exposes those weights in the observation stream.

  • Introduce ResourceRewardWeight observation and hook it into the reset lifecycle
  • Extend ResourceReward to sample weights in reset_pre_sim_init and publish them in reset_post_sim_init
  • Wire reset methods through Satellite, ObservationBuilder, and ActionBuilder and add corresponding tests

Reviewed Changes

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

Show a summary per file
File Description
tests/unittest/sats/test_access_satellite.py Add mocks for builders in reset test
tests/unittest/obs/test_observations.py Add unit tests for ResourceRewardWeight
tests/unittest/data/test_data.py Add tests for deterministic and randomized ResourceReward
tests/integration/obs/test_int_observations.py Add integration test for weight observation in multi-reward env
src/bsk_rl/sats/satellite.py Call builder reset hooks in Satellite.reset_* methods
src/bsk_rl/obs/observations.py Implement Resetable, add ResourceRewardWeight, builder resets
src/bsk_rl/data/resource_data.py Extend ResourceReward with randomized weight sampling
src/bsk_rl/act/actions.py Add reset hooks to ActionBuilder and Action
docs/source/release_notes.rst Document new feature in release notes
Comments suppressed due to low confidence (3)

tests/unittest/sats/test_access_satellite.py:315

  • The test named test_reset_pre_sim_init calls reset_overwrite_previous instead of reset_pre_sim_init; update the call to match the test’s intent.
sat.reset_overwrite_previous()

tests/unittest/data/test_data.py:344

  • This test computes reward but does not assert its value; add an assertion to verify the calculated reward matches the expected output.
reward = dm.calculate_reward(

src/bsk_rl/obs/observations.py:513

  • The signature indicates returning a float but the implementation returns a NumPy array; consider updating the return annotation to -> np.ndarray or similar for clarity.
def get_obs(self) -> float:

@Mark2000
Mark2000 force-pushed the feature/reward-randomization branch from f0062f2 to eaf1981 Compare June 12, 2025 16:26

@LorenzzoQM LorenzzoQM 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.

Very nice addition! Code looks good.

@Mark2000
Mark2000 force-pushed the feature/reward-randomization branch from eaf1981 to 448d4a5 Compare June 12, 2025 19:05
@Mark2000
Mark2000 merged commit cba0ce7 into develop Jun 12, 2025
5 checks passed
@Mark2000
Mark2000 deleted the feature/reward-randomization branch June 12, 2025 19:21
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