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

Fix doc building in CI #6884

Closed
wants to merge 9 commits into from
Closed

Fix doc building in CI #6884

wants to merge 9 commits into from

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Nov 1, 2022

Fixes #6875.

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 2, 2022

@NicolasHug
Copy link
Member

Thanks for the PR @pmeier . The related VideoReader issue seems a bit fishy #6875 (comment), I think we should figure out whether there's something wrong with it before committing to removing -j, which is fairly useful to make the doc build fast

@@ -33,6 +33,7 @@ clean:
rm -rf $(SOURCEDIR)/auto_examples/ # sphinx-gallery
rm -rf $(SOURCEDIR)/gen_modules/ # sphinx-gallery
rm -rf $(SOURCEDIR)/generated/ # autosummary
rm -rf $(SOURCEDIR)/models/generated # autosummary
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was missing

Comment on lines -1 to -5
from typing import Any, Dict, Iterator

import torch

from ..utils import _log_api_usage_once
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are no longer needed. flake8 didn't pick up on this since we blanket ignore F401, i.e. unused imports, in __init__.py files:

vision/setup.cfg

Lines 14 to 15 in c4c0ef9

per-file-ignores =
__init__.py: F401, F403, F405

@@ -43,8 +37,6 @@
"_read_video_timestamps_from_memory",
"_probe_video_from_memory",
"_HAS_VIDEO_OPT",
"_read_video_clip_from_memory",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are no longer available.

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 2, 2022

Although the build hangs in the reading sources... stage, the error likely happens during the gallery build. Removing plot_video_api.py (or prefixing it with an _) resolved the issue with multiprocessing activated.

I'll investigate further what exactly is causing the issue.

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 2, 2022

Narrowed it down to

import itertools
video.set_current_stream("video")
frames = [] # we are going to save the frames here.
# We seek into a second second of the video and use islice to get 10 frames since
for frame, pts in itertools.islice(video.seek(2), 10):
frames.append(frame)
print("Total number of frames: ", len(frames))

Behavior can be reproduced by replacing this with

video.set_current_stream("video")

for _ in video.seek(2):
    pass

and commenting out the rest of file.

@jdsgomes @bjuncek any idea what your PR changed that could interfere with sphinx multiprocessing?

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 2, 2022

I've pushed the reproduction to this PR. To reproduce locally, check this PR out and do

cd docs
pip install -r requirements.txt
make clean
make html

You probably also want to apply sphinx-doc/sphinx#10952 locally so using ctrl+c to terminate the hanging build actually terminates all sub-processes.

@jdsgomes
Copy link
Contributor

jdsgomes commented Nov 3, 2022

build_docs is green now, will trigger rebuild to see it it is flaky

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 3, 2022

It is still red on main: https://app.circleci.com/pipelines/github/pytorch/vision/21716/workflows/d4a070b9-cdb0-4929-a518-56e8b43fbe20/jobs/1759489. I'll try to reproduce again locally, but I could do so without issues yesterday.

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 3, 2022

It seems I reduced it down to much. Trying to reproduce it again with the reduced version today did no longer work. No idea why it worked yesterday. Sorry for that.

However, reverting the file back to its full content surfaces the error again. I pushed the change to this PR. Docs CI is red the same as on main now: https://app.circleci.com/pipelines/github/pytorch/vision/21733/workflows/d97d374f-03f2-4795-bf51-6eadf8daa235/jobs/1760798

@pmeier
Copy link
Collaborator Author

pmeier commented Nov 4, 2022

The offending PR was reverted in #6908. Thus, I'm closing this.

From the investigation with @jdsgomes, the missing piece to reproduce this was a CPU only installation of PyTorch. Apart from that, CI uses

  • python=3.7
  • ffmpeg=4.2
  • av<10

@pmeier pmeier closed this Nov 4, 2022
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.

build_docs fails with [Makefile:42: html] Hangup
4 participants