Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optimizer overrides to explorer #1604

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

odjuricicTT
Copy link
Contributor

@odjuricicTT odjuricicTT commented Dec 16, 2024

TT-explorer now supports editing output tensor layout of ops from the UI.

Changes made:

  • Create editable fields for op output layout instead of readonly attributes.
  • Keep perf trace as state for every executed model on the backend
  • Clean up location parsing logic
  • Fix a few bugs in override parsing / handling

Closes #1178

Comment on lines 413 to 421
memory_layout = layout.memory_layout_as_int
if memory_layout is not None:
result.append(
graph_builder.KeyValue(
key="memory_layout",
value=str(ttnn.TensorMemoryLayout(memory_layout)),
make_editable_kv(
graph_builder.KeyValue(
key="Tensor Memory Layout",
value=str(ttnn.TensorMemoryLayout(memory_layout)),
),
editable={
"input_type": "value_list",
"options": [str(o) for o in ttnn.TensorMemoryLayout],
},
)
)
Copy link
Contributor

@madcampos madcampos Dec 16, 2024

Choose a reason for hiding this comment

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

I've tested specifically the memory_layout attribute and it is not returning the editable property to the UI. 😕

For reference, here is where the editable attribute is expected in the response:

{
    "graphs": [
        {
            // ...
            "nodes": [
                {
                    // ...
                    "attrs": [
                        {
                            "key": "",
                            "value": "",
                            "editable": {} // <= Here is the editable property
                        },
                        // ...
                    ]
                },
                // ...
            ]
        }
    ]
}

Here are the available types for the editable attribute: https://github.com/tenstorrent/model-explorer/blob/main/src/ui/src/components/visualizer/common/input_graph.ts#L156-L178

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm that is strange. For me the editable property exists and it is displayed as editable in the UI. See screenshot below:
Screenshot 2024-12-17 at 14 43 16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested on this PR. Editable should be passed on convert command that comes after execute command is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem that i am hitting after this is that even when i change the value in the editable in the UI it is not sent in settings. Settings is equal to: {'optimizationPolicy': 'DF Sharding'}

Copy link
Contributor

Choose a reason for hiding this comment

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

So, the way it is working now, when you click the upload button, it will send the changes to the server and then clean up the changes.

Also, it is not sending the changes to either the execute command nor the convert command that comes after.

I'll make a change to send it on both commands

Copy link
Contributor

Choose a reason for hiding this comment

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

@odjuricicTT please check this branch: tenstorrent/model-explorer#19

@odjuricicTT odjuricicTT force-pushed the odjuricic/explorer-overrides branch from e959b38 to 58bb8aa Compare December 19, 2024 15:50
@odjuricicTT odjuricicTT force-pushed the odjuricic/explorer-overrides branch 2 times, most recently from ab70071 to d9a8568 Compare December 27, 2024 15:27
@odjuricicTT odjuricicTT force-pushed the odjuricic/explorer-overrides branch from d9a8568 to b394618 Compare December 27, 2024 15:28
@odjuricicTT odjuricicTT changed the title [WIP] Add optimizer overrides to explorer Add optimizer overrides to explorer Dec 27, 2024
@odjuricicTT odjuricicTT marked this pull request as ready for review December 27, 2024 16:00
@odjuricicTT
Copy link
Contributor Author

@vprajapati-tt I did make some changes to how location is parsed and used. We might be missing functionality here to cover all the usecases discussed. Let's revisit this after the holidays.

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.

Add overrides to explorer backend
3 participants