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

Docs examples #46

Merged
merged 10 commits into from Mar 19, 2020
Merged

Conversation

mattuntergassmair
Copy link
Collaborator

@mattuntergassmair mattuntergassmair commented Mar 10, 2020

improved examples in docs

work in progress, overlay tutorial still missing

as part of this PR, some defaults were changed:

  • default render mode is :basic rather than :fancy
  • default entity color determined based on vehicle id rather than COLOR_CAR_OTHER

I think the new defaults make more sense but this may be subjective and can be discussed / changed before closing the PR

@coveralls
Copy link

coveralls commented Mar 10, 2020

Coverage Status

Coverage increased (+5.3%) to 53.576% when pulling d37c011 on mattuntergassmair:docs_examples into ecefe3a on sisl:master.


You can use Reactive to have them endlessly drive in real time in your browser.

### TODO: is this a thing? If yes, provide snippet
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've never used Reactive for visualizing simulations, is this a thing?

Copy link
Member

Choose a reason for hiding this comment

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

I never used it either

Copy link
Member

Choose a reason for hiding this comment

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

we can probably remove that sentence

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, then maybe I'll leave it at a honorable mention of the package instead of providing a working code example.

src/AutoViz.jl Show resolved Hide resolved
@@ -227,7 +227,8 @@ function render_round_rect(

if fstroke
set_source_rgba(ctx, color_stroke)
line_width = abs(user_to_device_distance!(ctx, [line_width,0.0])[1])
# from Cairo.user_to_device_distance! - this may be the bug
# line_width = abs(user_to_device_distance!(ctx, [line_width,0.0])[1])
set_line_width(ctx, line_width)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cairo.user_to_device_distance may being used incorrectly here (line_width seems to vary based on heading angle of the vehicle). To be investigated further.

Copy link
Member

Choose a reason for hiding this comment

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

Yes it is not clear to me why we need this function.
I am not sure I understand what it does though: https://www.cairographics.org/manual/cairo-Transformations.html#cairo-user-to-device-distance

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that typically the line_width does not scale with the zoom level, so it becomes relatively "thinner" as one zooms in. And the user_to_device_distance is used to compensate for that, but maybe there's a bug. I'll open a separate issue so we can take care of it separately.

src/renderable.jl Outdated Show resolved Hide resolved

models = Dict((i => Tim2DDriver(timestep) for i in 1:3)) # car models
# TODO: use a different model for pedestrian
models[42] = Tim2DDriver(timestep) # pedestrian model
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what is a good pedestrian model to use here?

Copy link
Member

Choose a reason for hiding this comment

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

I would use StaticDriver to keep it simple

@MaximeBouton
Copy link
Member

Thanks for doing this, two main comments:

1 - Are you re designing brand new tutorials? I had made a notebook for AutoViz 0.8 that you can take inspiration from
2 - the notebook was used in the tests, it would be nice to have all the examples run in the test suite. I don't think there is a nice way to do it though. Maybe the easiest is to just convert some of the examples into proper tests.

@mattuntergassmair
Copy link
Collaborator Author

Thanks for doing this, two main comments:

1 - Are you re designing brand new tutorials? I had made a notebook for AutoViz 0.8 that you can take inspiration from
2 - the notebook was used in the tests, it would be nice to have all the examples run in the test suite. I don't think there is a nice way to do it though. Maybe the easiest is to just convert some of the examples into proper tests.

The examples are heavily inspired by the notebooks that were there in the notebooks folder (in fact, I exported them first and used them as a basis to start from). I tried not to lose any information, but felt that it made sense to restructure the examples significantly and focus on different aspects in different tutorials. For example, the camera tutorial now shows all the available cameras, and finally implements a custom camera. I just had a look at the notebook you mentioned, for some reason it wasn't there on the branch that I forked from. I'll make sure to integrate all the information from there in the docs (section on overlays is still work in progress)

@mattuntergassmair
Copy link
Collaborator Author

Thanks for doing this, two main comments:

1 - Are you re designing brand new tutorials? I had made a notebook for AutoViz 0.8 that you can take inspiration from
2 - the notebook was used in the tests, it would be nice to have all the examples run in the test suite. I don't think there is a nice way to do it though. Maybe the easiest is to just convert some of the examples into proper tests.

As for testing, how does it work at the moment? Is the documentation built automatically when we push changes? Could we make it fail when there are errors in @examples (currently only warnings). Or is there a way to convert *.md files to *.jl files programatically, such that we could use them for tests?

If none of the above works we should consider writing tests separately but I'm afraid there may be code duplication - would be nice if we could avoid that with one of the aforementioned options.

@MaximeBouton
Copy link
Member

Yes it is only warnings for now.
With the notebooks, the test suite was including the notebooks and running them so if there was a bug in the notebook it would break the test. Another extra is that it would reflect in the code coverage.
Maybe we can ask on the documenter repo if there is a way to include the examples in the tests.

@MaximeBouton
Copy link
Member

This is super neat!! thanks a lot

@MaximeBouton
Copy link
Member

Is this ready to merge? thanks for the work!

@mattuntergassmair mattuntergassmair merged commit dc84cf3 into sisl:master Mar 19, 2020
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.

None yet

3 participants