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

Interactively explore Models using ipywidgets #2061

Merged
merged 21 commits into from Jun 28, 2023
Merged

Interactively explore Models using ipywidgets #2061

merged 21 commits into from Jun 28, 2023

Conversation

sdrave
Copy link
Member

@sdrave sdrave commented May 25, 2023

This PR adds an interact(model, parameter_space) method to pymor.models.interact which, in a jupyter environment, sets up a gui to interactively visualize a Model's solution and/or output based on the given parameters.

Further, the matplotlib-based visualizer is extended to be able to return an interactive widget instead of a static display of the solution.

@sdrave sdrave added the pr:new-feature Introduces a new feature label May 25, 2023
@sdrave sdrave added this to the 2023.1 milestone May 25, 2023
@github-actions
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updateded dependencies.

@github-actions github-actions bot added the infrastructure Buildsystem, packages and CI label May 26, 2023
@sdrave sdrave self-assigned this May 31, 2023
@HenKlei HenKlei self-requested a review May 31, 2023 12:39
@lbalicki
Copy link
Member

lbalicki commented Jun 1, 2023

Works nicely for me! The auto-update was a bit too slow to follow my inputs but maybe that's just my slow computer. For time-dependent models with inputs: Would it be reasonable to have a textbox for the input function and then an ExpressionFunction is created from that?

@HenKlei
Copy link
Member

HenKlei commented Jun 2, 2023

When playing around with the auto update feature, after some time I get the following error:

.../lib/python3.10/site-packages/tornado/iostream.py", line 182, in advance
        assert 0 < size <= self._size

Apart from that, this looks quite nice.

@lbalicki The auto update was fast enough for me, I guess it really depends on how fast your computer can solve the underlying problem.

@sdrave
Copy link
Member Author

sdrave commented Jun 7, 2023

Works nicely for me! The auto-update was a bit too slow to follow my inputs but maybe that's just my slow computer.

Yes. That's also the reason why I disabled it by default.

For time-dependent models with inputs: Would it be reasonable to have a textbox for the input function and then an ExpressionFunction is created from that?

That should be easily possible. @pmli, would you agree that this is a good approach?

@pmli
Copy link
Member

pmli commented Jun 11, 2023

When playing around with the auto update feature, after some time I get the following error:

.../lib/python3.10/site-packages/tornado/iostream.py", line 182, in advance
        assert 0 < size <= self._size

I very quickly get into the same issue (just making 2-3 changes is enough).

For time-dependent models with inputs: Would it be reasonable to have a textbox for the input function and then an ExpressionFunction is created from that?

That should be easily possible. @pmli, would you agree that this is a good approach?

That sounds like a good idea to me.

@sdrave
Copy link
Member Author

sdrave commented Jun 14, 2023

@pmli, can you confirm that the error only occurs with jupyter notbook not jupyterlab? At least, that seems to be the case for @HenKlei.

@github-actions
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@github-actions
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@sdrave
Copy link
Member Author

sdrave commented Jun 16, 2023

I think this is now feature complete. I tried to improve the layout, but if some css expert wants to take a look, I'd be happy ... However, there is only so much we can do due to matplotlib/ipympl#297

@pymor/all, please take another look, how this works for you. (I'm not sure what I can do regarding the tornado issues ...)

@sdrave sdrave marked this pull request as ready for review June 16, 2023 15:48
@sdrave sdrave requested a review from pmli June 16, 2023 15:49
@pymor pymor deleted a comment from github-actions bot Jun 22, 2023
@pymor pymor deleted a comment from github-actions bot Jun 22, 2023
@sdrave
Copy link
Member Author

sdrave commented Jun 22, 2023

pre-commit.ci autofix

@pymor pymor deleted a comment from github-actions bot Jun 22, 2023
@github-actions
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@sdrave sdrave requested a review from lbalicki June 27, 2023 13:12
@sdrave
Copy link
Member Author

sdrave commented Jun 27, 2023

@lbalicki , @pmli any idea what is going on here with the test failure? Have you seen it before? There is a minor version update of pytest, however I'm not sure if that the reason of the test failure.

@pmli
Copy link
Member

pmli commented Jun 27, 2023

The issue seems to be that samdp doesn't compute residues that are exact complex conjugates for a complex conjugate pair of poles for that one case (I'm not sure why it didn't happen before). This then makes the dominance criterion different for complex conjugate poles and breaks the assumption in the sorting.

Changing the tol in the test case from 1e-12 to 1e-13 makes it work for me, but a proper fix in samdp or MTReductor would be better (maybe for the next release).

@sdrave
Copy link
Member Author

sdrave commented Jun 27, 2023

I see. I am wondering: Has this ever occurred before? Or could this actually be due to some stability issue on the runner? The Windows/macos runners do not seem to be super reliable in general. I will restart the build a few times and see what happens.

@sdrave
Copy link
Member Author

sdrave commented Jun 27, 2023

I see. I am wondering: Has this ever occurred before? Or could this actually be due to some stability issue on the runner? The Windows/macos runners do not seem to be super reliable in general. I will restart the build a few times and see what happens.

Ok, so the issue is non-deterministic ...

Copy link
Member

@pmli pmli left a comment

Choose a reason for hiding this comment

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

It works well with Jupyter Lab (I need to resize the window for the plots to show up, but maybe that's only my issue). Time-dependent parameters also work nicely.

docs/source/tutorial_interact.md Outdated Show resolved Hide resolved
@sdrave
Copy link
Member Author

sdrave commented Jun 27, 2023

It works well with Jupyter Lab (I need to resize the window for the plots to show up, but maybe that's only my issue).

That's strange. You get this issue on k3d without my fix that should enter the next release. However, the k3d backend should only be used when the yet-to-be-released version is detected. Can you confirm that you get the k3d visualization?

@pmli
Copy link
Member

pmli commented Jun 28, 2023

Can you confirm that you get the k3d visualization?

How do I do that? I think it doesn't work if I uninstall k3d, because reading the defaults file will fail.

@sdrave
Copy link
Member Author

sdrave commented Jun 28, 2023

Can you confirm that you get the k3d visualization?

How do I do that? I think it doesn't work if I uninstall k3d, because reading the defaults file will fail.

Eh, which defaults file? Are you using the one with which the docs are built? Then what you see is not surprising, as for the docs built k3d is enforced. You should just run the notebook without any specific defaults.

@pmli
Copy link
Member

pmli commented Jun 28, 2023

Can you confirm that you get the k3d visualization?

How do I do that? I think it doesn't work if I uninstall k3d, because reading the defaults file will fail.

Eh, which defaults file? Are you using the one with which the docs are built?

Yes.

Then what you see is not surprising, as for the docs built k3d is enforced. You should just run the notebook without any specific defaults.

Ok, that way, matplotlib is used.

Is it ok if the online docs shows a k3d plot?

@pymor pymor deleted a comment from github-actions bot Jun 28, 2023
@sdrave
Copy link
Member Author

sdrave commented Jun 28, 2023

Ok, that way, matplotlib is used.

Fine.

Is it ok if the online docs shows a k3d plot?

Well, I'd actually prefer if no output is shown there at all as the interface won't work anyway. Somehow the remove-output tag does not work for the first cells. (See the note at the top of the tutorial.)

@pmli
Copy link
Member

pmli commented Jun 28, 2023

Is it ok if the online docs shows a k3d plot?

Well, I'd actually prefer if no output is shown there at all as the interface won't work anyway. Somehow the remove-output tag does not work for the first cells. (See the note at the top of the tutorial.)

Aha, ok. It seems that a blank line is necessary between cell options and the code (I pushed the change).

@pymor pymor deleted a comment from github-actions bot Jun 28, 2023
@github-actions
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@sdrave sdrave enabled auto-merge June 28, 2023 18:30
@sdrave sdrave added this pull request to the merge queue Jun 28, 2023
@ftschindler
Copy link
Member

This is extremely nice work!

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 28, 2023
@pmli pmli added this pull request to the merge queue Jun 28, 2023
Merged via the queue into main with commit 319b731 Jun 28, 2023
19 checks passed
@pmli pmli deleted the model_interact branch June 28, 2023 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Buildsystem, packages and CI pr:new-feature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants