(Issue #99) Fix disc_episode_returns off-by-one error

[Katze2664]

self.disc_episode_returns should be calculated before incrementing self.episode_lengths

[ffelten]

Hi @Katze2664,

Thanks for the contribution 😄

Indeed, I think you are right. Could you also update the tests?

I don't really remember how I ended up with the values in the tests but they are not correct right now.

Cheers,

[ffelten]

up @Katze2664

[Katze2664]

Hi @ffelten, sorry for the delay in updating the tests. I don't have much experience with writing tests but I will have a go at updating them.

[LucasAlegre]

Looks good to me :)

[Katze2664]

Hi @ffelten, the test case wasn't too difficult to modify. I replaced the faulty assertion with
np.testing.assert_allclose(info["episode"]["dr"], [7.71538, -2.9109], rtol=0, atol=1e-2)
because I thought it was neater than

assert tuple(np.round(info["episode"]["dr"], 2)) == (
np.float32(7.72),
np.float32(-2.91),
)

but you can modify it if you disagree.

[Katze2664]

Perhaps this needs to be raised as it's own issue, but while I was fixing the test_mo_record_ep_statistic test case then I tried running the other test cases and found several failed. Does this happen when you run them too, or have I done something wrong?

Extract from test report:

=========================== short test summary info ===========================
FAILED tests/test_envs.py::test_all_env_api[mo-lunar-lander-continuous-v2] - ...
FAILED tests/test_envs.py::test_all_env_api[mo-supermario-v0] - OverflowError...
FAILED tests/test_envs.py::test_all_env_api[mo-swimmer-v4] - ValueError: XML ...
FAILED tests/test_envs.py::test_all_env_api[mo-reacher-v0] - ModuleNotFoundEr...
FAILED tests/test_envs.py::test_all_env_passive_env_checker[mo-lunar-lander-continuous-v2]
FAILED tests/test_envs.py::test_all_env_passive_env_checker[mo-supermario-v0]
FAILED tests/test_envs.py::test_all_env_passive_env_checker[mo-swimmer-v4] - ...
FAILED tests/test_envs.py::test_all_env_passive_env_checker[mo-reacher-v0] - ...
FAILED tests/test_envs.py::test_gymnasium_equivalence[Swimmer-v4-mo-swimmer-v4]
FAILED tests/test_envs.py::test_env_determinism_rollout[mo-lunar-lander-continuous-v2]
FAILED tests/test_envs.py::test_env_determinism_rollout[mo-swimmer-v4] - Valu...
FAILED tests/test_envs.py::test_env_determinism_rollout[mo-reacher-v0] - Modu...
================= 12 failed, 95 passed, 86 warnings in 49.23s =================

FAILED tests/test_envs.py::test_all_env_api[mo-lunar-lander-continuous-v2]

>           self.lander.ApplyLinearImpulse(
                (-ox * SIDE_ENGINE_POWER * s_power, -oy * SIDE_ENGINE_POWER * s_power),
                impulse_pos,
                True,
            )
E           TypeError: Converting from sequence to b2Vec2, expected int/float arguments index 0

mo_gymnasium\envs\lunar_lander\lunar_lander.py:162: TypeError

###############################################################################

FAILED tests/test_envs.py::test_all_env_api[mo-supermario-v0]

    @property
    def prg_rom_stop(self):
        """The exclusive stopping index of the PRG ROM."""
>       return self.prg_rom_start + self.prg_rom_size * 2**10
E       OverflowError: Python integer 1024 out of bounds for uint8

.venv\lib\site-packages\nes_py\_rom.py:198: OverflowError

###############################################################################

FAILED tests/test_envs.py::test_all_env_api[mo-swimmer-v4]

    def _initialize_simulation(
        self,
    ):
>       model = mujoco.MjModel.from_xml_path(self.fullpath)
E       ValueError: XML Error: Schema violation: unrecognized attribute: 'collision'
E       
E       Element 'option', line 3

.venv\lib\site-packages\gymnasium\envs\mujoco\mujoco_env.py:380: ValueError

###############################################################################

FAILED tests/test_envs.py::test_all_env_api[mo-reacher-v0]

>   from pybulletgym.envs.roboschool.envs.env_bases import BaseBulletEnv
E   ModuleNotFoundError: No module named 'pybulletgym'

mo_gymnasium\envs\reacher\reacher.py:6: ModuleNotFoundError

###############################################################################

FAILED tests/test_envs.py::test_all_env_passive_env_checker[mo-lunar-lander-continuous-v2]

>           self.lander.ApplyLinearImpulse(
                (-ox * MAIN_ENGINE_POWER * m_power, -oy * MAIN_ENGINE_POWER * m_power),
                impulse_pos,
                True,
            )
E           TypeError: Converting from sequence to b2Vec2, expected int/float arguments index 0

mo_gymnasium\envs\lunar_lander\lunar_lander.py:121: TypeError

###############################################################################```

[LucasAlegre]

@Katze2664 this is probably due to your version of Gymnasium. We are supporting Gymnasium 1.0 in another PR.