Skip to content

Fix: Gymnasium compliance and corner points detection bug#38

Draft
faustoallegrini wants to merge 3 commits intofloriankagerer:mainfrom
faustoallegrini:fix/bug-cp
Draft

Fix: Gymnasium compliance and corner points detection bug#38
faustoallegrini wants to merge 3 commits intofloriankagerer:mainfrom
faustoallegrini:fix/bug-cp

Conversation

@faustoallegrini
Copy link

This PR introduces two key fixes to improve compliance and correctness:

  1. Gymnasium API compliance

    • Explicitly handle seed parameter in reset(seed=...) for reproducible initialization
    • Pass custom attributes via options in reset(..., options=...) instead of positional arguments
    • Ensure step(...) returns the truncated flag along with (obs, reward, terminated, truncated, info)
  2. Corner points detection bug

    • Corrected logic error in the corner detection algorithm that caused incomplete detection
    • Ensured all valid corner points are properly identified

- Explicitly handle `seed` in `reset(seed=...)` to ensure reproducible initialization
- Pass custom attributes via `options` in `reset(..., options=...)` instead of positional/extra args
- Return `truncated` flag from `step(...)` alongside `(obs, reward, terminated, truncated, info)`

Reason:
Align environment behavior with Gymnasium API expectations:
- Deterministic seeding through the `seed` parameter
- Proper use of the `options` dict for extra reset configuration
- Complete step signature including the `truncated` signal

Refs:
Gymnasium env API: reset(seed=None, options=None); step(...) must return truncated.
- Fixed logic error that caused incomplete detection of corner points
- Ensured all valid corners are now properly identified

Reason:
Previous implementation missed some corners due to incorrect detection logic.
@floriankagerer floriankagerer self-assigned this Dec 13, 2025
@floriankagerer
Copy link
Owner

Hello faustoallegrini,
many thanks for your contribution to this repository.

I'm afraid that I need more time to carefully review your changes.
What I can say from a first look:

  1. Thanks for detecting a bug in the corner point detection. I'd really appreciate if you also add tests for the corner point detection (instead of print, please use logger.debug).
  2. Instead of using {} as default argument in the reset methods, please use None.
  3. Thanks for showing improvements regarding the Gymnasium compliance. To be honest, I just updated the import and did not take care of the methods, which were designed for gym.
  4. Regarding the updates of the preview items and selectable items: I agree with your argument, that there should not be a difference in the logic. However, my plan is to refactor this logic in two dedicated modules, so I'd prefer to keep it as it is now and to update the logic when implementing these modules.

Thanks again for your contribution.
I am looking forward to hearing again from you.

Best regards,
Florian

@floriankagerer floriankagerer marked this pull request as draft December 13, 2025 09:00
@faustoallegrini
Copy link
Author

Hello Florian,
Thank you for your feedback.
I created a fictitious, simplified order named “2orders-mikp” with a container size of 15×10 to provide a clearer visualization of the CPs. Running this order using the “LowestArea” heuristic helps cover several edge cases in the CP algorithm logic.
Additionally, I implemented a function to plot both the heightmap and the corresponding corner points. You can test this by running the script demo_gym_palenv.py.

Best regards,
Fausto

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.

2 participants