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

Improve elastic-tube-1d code and documentation #209

Merged
merged 12 commits into from
Apr 28, 2021

Conversation

IshaanDesai
Copy link
Member

@IshaanDesai IshaanDesai commented Apr 27, 2021

Related to #188 but in the context of the new tutorials structure. The elastic-tube-1d was ported to the tutorials from a separate repository and this PR is a check if all the post-processing steps work as expected.

Closes #188

@IshaanDesai IshaanDesai requested a review from MakisH April 27, 2021 12:07
@IshaanDesai IshaanDesai marked this pull request as ready for review April 27, 2021 12:07
elastic-tube-1d/README.md Show resolved Hide resolved
elastic-tube-1d/README.md Outdated Show resolved Hide resolved
elastic-tube-1d/plot-diameter.sh Show resolved Hide resolved
@IshaanDesai IshaanDesai changed the title Improve elastic-tube-1d visualization code and documentation Improve elastic-tube-1d code and documentation Apr 27, 2021
@IshaanDesai
Copy link
Member Author

Attempting to generate a video in FluidSolver.py by passing the flag --write-video leads to the following error:

MovieWriter stderr:
convert-im6.q16: delegate failed `'ffmpeg' -nostdin -v -1 -i '%M%%d.jpg' '%u.%m' 2> '%u'' @ error/delegate.c/InvokeDelegate/1928.

Traceback (most recent call last):
  File "./FluidSolver.py", line 161, in <module>
    writer.finish()
  File "/home/desaiin/.local/lib/python3.8/site-packages/matplotlib/animation.py", line 354, in finish
    self.cleanup()
  File "/home/desaiin/.local/lib/python3.8/site-packages/matplotlib/animation.py", line 390, in cleanup
    raise subprocess.CalledProcessError(
subprocess.CalledProcessError: Command '['convert', '-size', '640x480', '-depth', '8', '-delay', '6.666666666666667', '-loop', '0', 'rgba:-', 'writer_test.mp4']' returned non-zero exit status 1.
(0) 10:32:47 [impl::SolverInterfaceImpl]:147 in ~SolverInterfaceImpl: Implicitly finalizing in destructor

@MakisH could you please reproduce this to ensure it is not just something with my system? If this persists then I suggest temporarily suspending the video writing functionality and resolving this in another PR. This PR already has a lot of clean-up

@MakisH
Copy link
Member

MakisH commented Apr 28, 2021

python3 FluidSolver.py --enable-plot --write-video

works for me, I see the live animation. We should document this option.

I am using Python 3.8.6 and the Python binding v2.2.0.2.

@IshaanDesai
Copy link
Member Author

I see the live animation

I also see the live animation which is the succession of plots generated by matplotlib. But do you get a valid .mp4 file in the directory at the end of the simulation? I see this error at the very end.

@MakisH
Copy link
Member

MakisH commented Apr 28, 2021

I get a valid video file, here it is:

writer_test.mp4

I don't see any errors even after completing (fluid-python + solid-python).

Edit: GitHub does not show the video, but it works with every player I tried locally (GNOME Videos, VLC). Renaming it to something more decsriptive than writer_test.mp4 would be good, though.

video.tar.gz

@IshaanDesai
Copy link
Member Author

I get a valid video file

Okay great, in that case I add the --write-video flag information to the README and this PR is ready to be merged.

@IshaanDesai IshaanDesai requested a review from MakisH April 28, 2021 11:29
@MakisH
Copy link
Member

MakisH commented Apr 28, 2021

Maybe already try fixing the failing "Lint docs" pipeline to experience these new checks, it should be very easy. :-)

@IshaanDesai
Copy link
Member Author

Maybe already try fixing the failing "Lint docs" pipeline to experience these new checks, it should be very easy. :-)

Interestingly doing act -j check_md passes on my local machine but somehow fails in the remote deployment! Have you experienced this before?

@MakisH
Copy link
Member

MakisH commented Apr 28, 2021

Interestingly doing act -j check_md passes on my local machine but somehow fails in the remote deployment! Have you experienced this before?

I have not yet had the chance to try out act, so no clue here. But I use the davidanson.vscode-markdownlint extension in VSCode and it shows the same errors (since all exceptions are already in the files or in the project markdownlint config.

@IshaanDesai
Copy link
Member Author

I have not yet had the chance to try out act, so no clue here. But I use the davidanson.vscode-markdownlint extension in VSCode and it shows the same errors (since all exceptions are already in the files or in the project markdownlint config.

Thanks! davidanson.vscode-markdownlint extension caught the problem even though act -j check_md did not catch it.

@IshaanDesai IshaanDesai merged commit 21fd7fa into develop Apr 28, 2021
@IshaanDesai IshaanDesai deleted the clean-visualization branch April 28, 2021 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants