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.

Sign up for GitHub

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

Jump to bottom

Add vertexing tutorial #223

Merged
merged 13 commits into from
Jun 3, 2020
Merged

Add vertexing tutorial #223

merged 13 commits into from
Jun 3, 2020

Conversation

baschlag
Copy link
Contributor

This PR adds a vertexing tutorial MD file as well as dedicated files for writing the tutorial code in.

@baschlag baschlag added the Improvement Changes to an existing feature label May 27, 2020
@acts-issue-bot acts-issue-bot bot removed the Triage label May 27, 2020
@baschlag baschlag added this to the v0.26.00 milestone May 27, 2020
@baschlag baschlag requested a review from msmk0 May 27, 2020 16:45
@codecov
Copy link

codecov bot commented May 27, 2020 •
edited
Loading

Codecov Report

Merging #223 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #223   +/-   ##
=======================================
  Coverage   44.99%   44.99%           
=======================================
  Files         374      374           
  Lines       18697    18697           
  Branches     8842     8842           
=======================================
  Hits         8413     8413           
  Misses       4906     4906           
  Partials     5378     5378

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 256ab0e...e1bbcba. Read the comment docs.

@baschlag
Copy link
Contributor Author

@paulgessinger I've adapted the code after our discussion and made it clear that TutorialAMVFAlgorithm is only a skeleton file to be filled in the tutorial that I've linked in the code. This PR now also introduces a "normal" AMVF example (since this was missing anyways) which always gets built and can be used as reference code for the tutorial.

msmk0
msmk0 suggested changes May 28, 2020
View reviewed changes
Copy link
Contributor

@msmk0 msmk0 left a comment

Choose a reason for hiding this comment

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

The tutorial looks good.

I am not sure if we should add the empty tutorial example. I think it is sufficient to have the full example and the detailed description of what each part does in the permanent howto. For the tutorial it made sense to have the empty file but for the permanent version I do not think it does.

Examples/Algorithms/Vertexing/src/FullExampleAMVFAlgorithm.cpp Outdated Show resolved Hide resolved
Examples/Algorithms/Vertexing/src/TutorialAMVFAlgorithm.cpp Outdated Show resolved Hide resolved
Examples/Algorithms/Vertexing/src/TutorialAMVFAlgorithm.cpp Show resolved Hide resolved
Examples/Run/Vertexing/CMakeLists.txt Outdated Show resolved Hide resolved
docs/howto/setup_and_run_vertexing.md Show resolved Hide resolved
docs/howto/setup_and_run_vertexing.md Show resolved Hide resolved
docs/howto/setup_and_run_vertexing.md Show resolved Hide resolved
@msmk0 msmk0 mentioned this pull request May 29, 2020
Closed
@baschlag baschlag requested a review from msmk0 June 2, 2020 13:17
msmk0
msmk0 approved these changes Jun 2, 2020
View reviewed changes
Copy link
Contributor

@msmk0 msmk0 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the changes. The remaining points are only nitpicking from my side.

Examples/Algorithms/Vertexing/src/TutorialAMVFAlgorithm.cpp Show resolved Hide resolved
docs/howto/setup_and_run_vertexing.md Outdated Show resolved Hide resolved
@baschlag
Copy link
Contributor Author

baschlag commented Jun 2, 2020

Thanks @msmk0, I've updated the docs after your last suggestion. Should be ready from my side.

@msmk0
Copy link
Contributor

msmk0 commented Jun 2, 2020

Thanks @msmk0, I've updated the docs after your last suggestion. Should be ready from my side.

Thanks, I will merge this once the CI builds finish.

@msmk0
Copy link
Contributor

msmk0 commented Jun 3, 2020

@baschlag Could you rebase the PR?

baschlag added 13 commits June 3, 2020 11:31
@baschlag
first commit adding ACTS vertexing tutorial
4c72d61
@baschlag
add tutorial like amvf version, still seg faults...
d1f497e
@baschlag
fix format and add reference full tutorial code
4ca8920
@baschlag
add howto vertexing tutorial
e3de88c
@baschlag
update copyright
c5b2f15
@baschlag
update copyright pt 2
efa12e1
@baschlag
update vertexing tutorial documentation
f35d2b8
@baschlag
rename ExampleAMVFAlgorithm to TutorialExampleAMVFAlgorithm and intro…
fca9eed 
…duce full example AMVF
@baschlag
fix format
eec32b9
@baschlag
rename TutorialExampleAMVFAlgorithm -> TutorialAMVFAlgorithm
368cdaa
@baschlag
update include statements and docs to point to docs/howto/setup_and_r…
eb0a745 
…un_vertexing.md file
@baschlag
rename VertexFindingAlgorithm to IterativeVertexFinderAlgorithm and F…
86245e9 
…ullExampleAMVFAlgorithm to AdaptiveMultiVertexFinderAlgorithm
@baschlag
small update to setup_and_run_vertexing.md
e1bbcba
@baschlag
Copy link
Contributor Author

baschlag commented Jun 3, 2020

@baschlag Could you rebase the PR?

Done.

@msmk0 msmk0 merged commit 661c107 into acts-project:master Jun 3, 2020
@baschlag baschlag deleted the vertex_tutorial branch June 23, 2020 12:10
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 13, 2020
@baschlag @paulgessinger
Add vertexing tutorial (acts-project#223)
5e152f7
@AJPfleger AJPfleger mentioned this pull request Jan 31, 2024
Merged
kodiakhq bot pushed a commit that referenced this pull request Feb 1, 2024
@AJPfleger
fix: remove outdated vertexing tutorial (from #223) (#2915)
e4bc1f1 
closes #2900

Some parts of the tutorial are already missing. It was added some years ago for an event with #223.
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
@AJPfleger @LaraCalic
fix: remove outdated vertexing tutorial (from acts-project#223) (acts…
64f8a3e 
…-project#2915)

closes acts-project#2900

Some parts of the tutorial are already missing. It was added some years ago for an event with acts-project#223.
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
@AJPfleger @EleniXoch
fix: remove outdated vertexing tutorial (from acts-project#223) (acts…
f4556f9 
…-project#2915)

closes acts-project#2900

Some parts of the tutorial are already missing. It was added some years ago for an event with acts-project#223.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msmk0 msmk0 msmk0 approved these changes

Assignees
No one assigned
Labels
Improvement Changes to an existing feature
Projects
None yet
Milestone
v0.26.00
Development

Successfully merging this pull request may close these issues.
2 participants
@baschlag @msmk0