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

Enable ZX Live to be run from inside of a notebook. #161

Merged
merged 5 commits into from
Nov 15, 2023

Conversation

dlyongemallo
Copy link
Contributor

No description provided.

@jvdwetering
Copy link
Collaborator

This is very cool!
It is a little bit confusing when you open up Proof mode and try to save though, because then it still thinks it is embedded and it just doesn't do anything. If you click File->New also nothing happens. I'm not sure what the correct behaviour should be. I think allowing the user to work as much as possible as in regular ZXLive is probably the way to go.

@jvdwetering
Copy link
Collaborator

Right now the instance of the graph in ZXLive and that in pyzx are the same. So if you make a change in ZXLive, then this is also reflected in pyzx, which is what you want. However, you can also make a change in pyzx, and this also changes the graph in ZXLive, but this is not reflected in the visuals. It is probably too hard to make ZXLive just 'know' when a change happened, but maybe we can add a command like 'update_graph' that just updates all the graphics, which you can call when you are done doing stuff in pyzx?

@jvdwetering
Copy link
Collaborator

jvdwetering commented Nov 2, 2023

This could maybe even the form of an environment, like:

with zxl.update:
    zx.spider_simp(g)

Or perhaps even better, whenever the ZXLive window regains focus, we call a function that redraws the graph.

@dlyongemallo
Copy link
Contributor Author

It is a little bit confusing when you open up Proof mode and try to save though, because then it still thinks it is embedded and it just doesn't do anything. If you click File->New also nothing happens. I'm not sure what the correct behaviour should be.

What is the intended workflow when a user opens ZX Live from a notebook to edit a graph, and then opens Proof mode? Is it intended that the final state of the graph after the proof steps be saved back into the notebook? What shouldn't happen is that two tabs (the original edit panel plus the new proof panel) both point back to the original graph, as it's then confusing if the user makes changes to both and then saves them.

I think only one tab should be connected to the original graph, in which case maybe the edit panel should be locked or disabled or even closed if the user opens a proof panel, when ZX Live in in embed mode. But maybe it would be confusing if the app behaved (too) differently in standalone mode vs. embedded mode.

(It's also necessary to handle the case where the user calls ZX Live to edit the same graph twice. This shouldn't be allowed, and should result in an error message, or maybe just bringing the existing tab into focus.)

I think allowing the user to work as much as possible as in regular ZXLive is probably the way to go.

I think the least confusing thing would be that each ZX Live window (not tab) is associated with a single graph from the notebook, and whatever you did inside that window is tied to the original graph that was used to open it. The way that Qt applications are implemented in ipython, there is a singleton instance of the app per kernel. And since ZX Live is implemented with one main window for the app, that means that each new graph is opened in a new tab inside that main window. It would have to be changed to allow opening multiple windows in addition to multiple tabs, if it's desirable to have one window per original notebook graph object. Or maybe it's fine to just have different graphs open in different tabs, the way it is now.

@dlyongemallo
Copy link
Contributor Author

Right now the instance of the graph in ZXLive and that in pyzx are the same. So if you make a change in ZXLive, then this is also reflected in pyzx, which is what you want.

It's the "same" only in the sense that if you make a change in ZX Live and click File->Save, the changes are saved back to the graph in the notebook. But the changes are not "live". I think it's less confusing/dangerous if no data is actually changed in the notebook until/unless the user deliberately requests the changes be saved.

It is possible to make it so that changes made in ZX Live are immediately saved back to the notebook. In graphscene.py, there is a line that makes the assignments new_g = diff.apply_diff(self.g), self.g = new_g. If, instead of doing that, it did what's currently done in the PR in handle_save_embedded_graph_action instead, the update will be written into the original graph in the notebook.

The problem is that ZX Live (or any Qt application) is opened asynchronously, so that the notebook continues to run. So the user could have both the notebook and ZX Live open, and make changes in both, and ZX Live is not aware of the changes made to the graph in the notebook.

However, you can also make a change in pyzx, and this also changes the graph in ZXLive, but this is not reflected in the visuals.

The PR is just a proof-of-concept but there are two ways to go with this. Either we make a copy of the graph initially when it's imported into ZX Live, to break the link explicitly (so that changes made in the notebook no longer affect anything in ZX Live, and the only way to send the edits back to the notebook is via File->Save); or we somehow try to detect or allow the user to inform us if the graph is changed in the notebook, and update the internal state and visuals in ZX Live.

I think the former is clearer/cleaner, and that the latter leaves many possible opportunities for bugs due to things being out of sync.

It is probably too hard to make ZXLive just 'know' when a change happened, but maybe we can add a command like 'update_graph' that just updates all the graphics, which you can call when you are done doing stuff in pyzx?

What's a use case in which a user edits a graph in ZX Live, but continues to make changes using pyzx in the notebook, and needs to update the opened graph in ZX Live? Instead of updating the graph, which is complicated, wouldn't it make more sense to just open a new tab with the changed graph?

@dlyongemallo
Copy link
Contributor Author

I'll experiment with keeping changes between the notebook and ZX Live in sync, but I suspect it's going to lead to weird edge cases, like what happens if the user makes changes to both and the update in both directions happen at the same time. There's just all kinds of possibilities for bugs because of race conditions.

@jvdwetering
Copy link
Collaborator

That's true, I didn't think of what would happen if you would make changes in both settings.

I think only one tab should be connected to the original graph
I agree with that.
And then when you open a proof panel, that the graph there should not be linked to the one in pyzx. Maybe a nice solution would be to make all the opened graphs in zxlive available via some variables to pyzx, so that you can get them out and do stuff with them in pyzx.
It is possible to make it so that changes made in ZX Live are immediately saved back to the notebook
I think like you said, we shouldn't do this, but only when the user explicitly saves.

If we want to go the route of syncing the ZXLive and pyzx graph so that a change in one updates the other, I think we should let the pyzx changes take precedence. So if I make some change in ZXLive, which I don't save, and then I make a change in pyzx, this should overwrite my changes in ZXLive. I think this is the cleanest way because the zxlive application is called by pyzx

@dlyongemallo
Copy link
Contributor Author

I'll experiment with keeping changes between the notebook and ZX Live in sync

... and it turns out to be a huge mess due to the ability to undo/redo. Any updates made in the notebook would have to also be saved on the undo stack. That's complex enough without introducing an external source of possible edits.

@dlyongemallo
Copy link
Contributor Author

Maybe a nice solution would be to make all the opened graphs in zxlive available via some variables to pyzx, so that you can get them out and do stuff with them in pyzx.

I think the solution which would lead to the fewest potential bugs down the road is to copy the input graph, work entirely on that within ZX Live, and return some kind of object to the caller from which they can extract additional information. The caller should be informed that this returned object can change due to actions taken in the embedded ZX Live app, but the connection to the original graph has been severed and they can do whatever they want with it. I think this solution has the clearest semantics which can be understood by a user.

That also solves the problem of what happens if you open the same graph object twice: ZX Live simply has two copies of the graph in different tabs, which don't affect each other. This can even be considered a feature. Maybe the user wants to compare different sets of edits (or proof steps or whatever) to the same original graph.

However, the downside is that the graph is copied, which requires memory. Is that an issue for larger graphs? (But I don't imagine that a graphical tool would be very useful for a graph with millions of vertices, for instances.)

The API design question here is then what kind of information is needed to be communicated from ZX Live back to the notebook? For example, the returned object can bundle the working copy of the input graph, as well as other things produced from it like proofs.

p.s. Is it "ZX Live" or "ZXLive"? I've seen both but it should be kept consistent in the documentation and also in the app's own dialogs.

@jvdwetering
Copy link
Collaborator

I think that solution sounds good. Something like zxlive.return_graph(tab_index). I would suggest then that this also gives a copy of the graph, and that you can explicitly replace the graph in ZXLive by something like zxlive.set_graph(tab_index). This would make the interaction very transparant.

The memory cost of the copying is not important, as I can't imagine you would want to use ZXLive for a reallly large graph.

@dlyongemallo
Copy link
Contributor Author

The tab_index isn't stable, in that if you open or close or rearrange the tabs, they get re-indexed. But we can certainly assign each input graph a unique ID within ZX Live, which can be passed to the user to manipulate that graph and its dependents inside ZX Live.

@dlyongemallo dlyongemallo marked this pull request as draft November 9, 2023 11:04
@dlyongemallo dlyongemallo marked this pull request as ready for review November 10, 2023 12:43
@dlyongemallo
Copy link
Contributor Author

Okay, I've added a way for the user to get (a copy of) the graph by name. Also, if the user edits a graph whose name already exists in a tab, that tab is replaced (but the action can be undo from within ZXLive).

However, I'm not sure what to do if the name is not unique. Right now, it just acts on the first tab with that name.

@jvdwetering jvdwetering merged commit 06fbad8 into zxcalc:master Nov 15, 2023
2 checks passed
@dlyongemallo dlyongemallo deleted the enable_zxlive_from_notebook branch November 15, 2023 11:18
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