support mapd #50

RenKoya1 · 2025-05-22T02:39:54Z

Hello. I'm Ren in Tokyo tech.
I enabled each agent to manage state and made it compatible with mapd.

JustinShetty · 2025-05-22T16:38:14Z

Hi Ren, this sounds promising. However, I'm having a little trouble understanding the logic changes because most of the files also have indentation and formatting changes. Could you reduce this PR to just modify the logic so I can better understand it?

RenKoya1 · 2025-05-23T02:17:07Z

Yes. I'll change it

RenKoya1 · 2025-06-10T06:55:47Z

Hey Justin.
Sorry for the late.
I removed formatting.

JustinShetty · 2025-07-02T04:47:46Z

README.md

+    -   **Pose**: Each `(...),` represents an agent's pose. The first two elements are the x and y coordinates respectively. The third element (e.g. `X_PLUS`) is optional if your solver considers orientation.
+
+> [!WARNING]
+> Please note that either **all** or **none** of the poses must contain orientation. A mix of orientation and orientation-less poses is not supported.


please update this warning to explain that all or none must contain orientation information and separately, all or none must contain MAPD state

JustinShetty · 2025-07-02T04:52:00Z

README.md


-> [!WARNING]
-> Please note that either **all** or **none** of the poses must contain orientation. A mix of orientation and orientation-less poses is not supported.
+#### 📦 MAPD Example (Pickup and Delivery)


it might be useful to give 4 examples for the format

no orientation with orientation

MAPF A B

MAPD C D

JustinShetty · 2025-07-02T04:53:28Z

README.md

-   npm run dev
-   ```
+    ```sh
+    npm run dev


Not a big deal, but it's odd that our editors formatted this differently. I wonder how I can standardize that for the project...

JustinShetty · 2025-07-02T04:53:59Z

public/demo_random-32-32-20.txt

Please separate this into a new demo file and retain the original as-is

JustinShetty · 2025-07-02T04:55:52Z

src/AnimationControl.tsx

 }

-export default AnimationControl;
+export default AnimationControl;


I just merged #51 which will enforce some new whitespace rules (requires newline at the end of the file and disallows trailing spaces), please pull the latest main and update the formatting as required (--fix with eslint is helpful for these)

JustinShetty · 2025-07-02T04:59:43Z

src/Solution.tsx

+export enum AgentState {
+  PICKING,
+  CARRYING,
+  DELIVERED,
+  IDLE,
+  NONE,
+}


I think in general the best practice is to list the null/none value first in the enum definition (i.e. so it has integer value 0)

JustinShetty · 2025-07-02T05:05:29Z

src/Solution.tsx

 export class Pose {
    public position: Coordinate = new Coordinate(0, 0);
    public orientation: Orientation = Orientation.NONE;
+    public state: AgentState = AgentState.NONE;


Pose is only the position and orientation of an agent, i.e. it is a kind of state.

Try renaming Pose to AgentState and rework state: AgentState into something like mapdState: MapdState

JustinShetty · 2025-07-02T05:05:47Z

src/Solution.tsx

+            const o = orientationFromString(match[3] || ""); 
+            const stateStr = match[4] || "NONE";
+
+            console.log(stateStr);


JustinShetty · 2025-07-02T05:08:07Z

src/Solution.tsx

+        if (config.length === 0) {
+            throw new Error("Invalid solution: no poses found");
        }
        if (config.length === 0) throw new Error("Invalid solution");


JustinShetty · 2025-07-02T05:14:13Z

src/PixiApp.tsx

-        // Goal markers
-        const goalMarkers = viewport.addChild(goalMarkersRef.current);
-        solution[solution.length - 1].forEach((pose, agentId) => {
-            const marker = goalMarkers.addChild(new PIXI.Graphics());
-            const width = GRID_UNIT_TO_PX / 4;
-            marker.rect(
-                scalePosition(pose.position.x) - width / 2,
-                scalePosition(pose.position.y) - width / 2,
-                width, width)
-            .fill(AGENT_COLORS[agentId % AGENT_COLORS.length]);
-        });


Are these completely removed from MAPF visualizations?

Oh, I think deliveredMarkersRef includes the MAPF and MAPD cases. If that's true, should goalMarkersRef be removed?

JustinShetty · 2025-07-02T05:19:08Z

src/PixiApp.tsx

+                    continue;
+                }
+
+            if (!deliveredPose && pose.state === AgentState.DELIVERED) {


fix indentation

JustinShetty · 2025-07-02T05:27:42Z

src/PixiApp.tsx

+        deliveredMarkersRef.current.removeChildren();
+
+        solution[0].forEach((_pose, agentId) => {
+            let lastPickingPose: (typeof solution)[0][0] | null = null;


This isn't necessarily the agent's last pickup, right? Is it more correct to say nextPickup ? It looks like the loop below scans the solution until it finds the next delivery/idle and optionally a pickup marker if one is present before the delivery/idle. Is my understanding correct?

In any case, this is a lot of computation to do every frame. For large visualizations this may have a substantial performance impact. Ideally, you would scan the solution once when it's initially loaded and work through a list of processed list of markers... Maybe it's ok because the loop breaks once it's found the necessary information, but I'm still not happy about looping over the solution more than we need to

JustinShetty

Thanks for your contribution. I really like this idea, but I have some concerns with how you're structuring agent state and how much overhead managing pickup/delivery markers is adding to every frame. All the markers are destroyed, then it searches the solution (could be quite large) for the next pickup (optional) and delivery for every agent between each frame. I think that aspect can be improved with some work.

support mapd

bb795cb

RenKoya1 added 2 commits June 10, 2025 15:29

remove format

862a1ee

remove format

58e7d5f

JustinShetty reviewed Jul 2, 2025

View reviewed changes

public/demo_random-32-32-20.txt

Copy link

Owner

JustinShetty Jul 2, 2025

Choose a reason for hiding this comment

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

Please separate this into a new demo file and retain the original as-is

JustinShetty reviewed Jul 2, 2025

View reviewed changes

src/PixiApp.tsx

continue;

}

if (!deliveredPose && pose.state === AgentState.DELIVERED) {

Copy link

Owner

JustinShetty Jul 2, 2025

Choose a reason for hiding this comment

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

fix indentation

JustinShetty reviewed Jul 2, 2025

View reviewed changes

JustinShetty requested changes Jul 2, 2025

View reviewed changes

support mapd #50

Are you sure you want to change the base?

support mapd #50

Uh oh!

Conversation

RenKoya1 commented May 22, 2025

Uh oh!

JustinShetty commented May 22, 2025

Uh oh!

RenKoya1 commented May 23, 2025

Uh oh!

RenKoya1 commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JustinShetty Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JustinShetty Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JustinShetty left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JustinShetty Jul 2, 2025 •

edited

Loading

JustinShetty Jul 2, 2025 •

edited

Loading

JustinShetty left a comment •

edited

Loading