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

Add CMS Getting Started for AOD, MiniAOD, NanoAOD #3551

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

jmhogan
Copy link
Contributor

@jmhogan jmhogan commented Jan 31, 2024

This PR adds docs pages for CMS AOD, MiniAOD, and NanoAOD formats, addressing several tasks in #3452.

  • cernopendata/modules/fixtures/data/docs/cms-getting-started-aod/ will replace the cms-getting-started-2010 and cms-getting-started-2011 pages
  • cernopendata/modules/fixtures/data/docs/cms-getting-started-miniaod/ will replace the cms-getting-started-2015 page
  • cernopendata/modules/fixtures/data/docs/cms-getting-started-nanoaod/ is a new addition

Please forgive the inevitable first-time-contributor gaffs!

Some of the contents of these pages refer to records that will be made in the future. Hopefully all links are currently valid, but several should updated with new records before new CMS documentation are records are put into production. (#3495, #3496, #3537, #3281, #3279)

This slightly addresses #3465 by no longer linking to these older records as examples.

Copy link
Member

@katilp katilp Jan 31, 2024

Choose a reason for hiding this comment

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

If I'm not wrong the standard mkedanlz does not produce code that works in CMSSW_10_6_30. It is not the container feature but the release feature: it includes objects that are not in the standard miniaod (and also does not produce the config file). A mention of that could be added. A work-through attempt is reported in the GitLab issue in https://gitlab.cern.ch/cms-cloud/cmssw-docker-opendata/-/issues/18#note_7141873)

The current MiniAOD mkedanlz instructions work in 7_6_7 container, so that correct.

Copy link
Member

@katilp katilp left a comment

Choose a reason for hiding this comment

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

Looks great! After a quick reading, the only point I have is to add a mention that the 2016 MiniAOD mkedanlz does not work as such.

@jmhogan
Copy link
Contributor Author

jmhogan commented Jan 31, 2024

Ok, @katilp do we not have the same "hack" in that container that we do in 767? If not, it would be useful to add it in the upcoming refreshes...

ETA: plan is to recheck all the commands today in 10-6h30, there was some issues with finding enough disk space for the container...

@katilp
Copy link
Member

katilp commented Jan 31, 2024

Ok, @katilp do we not have the same "hack" in that container that we do in 767? If not, it would be useful to add it in the upcoming refreshes...

This a different problem, it is not just to make it work: it works but provides a non-working example
So some heavier hacking would be needed. I'd go for a git repository with a basic equivalent skeleton

@jmhogan
Copy link
Contributor Author

jmhogan commented Jan 31, 2024 via email

@jmhogan
Copy link
Contributor Author

jmhogan commented Feb 1, 2024

@katilp new commits change up the EDAnalyzer example in MiniAOD:

  • 2015 container pointed to mkedanlzr
  • all others (2016 container, either VM) pointed to clone this: https://github.com/cms-opendata-analyses/MiniAnalyzer_CMSSW_10_6_30
    Tested in the 2016 container -- the analyzer itself is a direct copy of mkedanlzr in the 7_6_7 container. I haven't tested the VMs but would be surprised if it doesn't work as stated.

I also (for now) have removed the note that cmsenv should not be done in the container.

I need to triple check the rendering tomorrow on my desktop where I've worked up until now. I'm getting HTTPS Connection refused messages on my laptop when I run any of the instance cleaning or populating commands, though build, up, and down commands are successful.

To be seen whether I can pass the checks...

@jmhogan
Copy link
Contributor Author

jmhogan commented Feb 1, 2024

Ok, ready to go on my side.

@katilp
Copy link
Member

katilp commented Feb 1, 2024

Perfect, thanks!

@tiborsimko
Copy link
Member

@jmhogan @Ari-mu-l Thanks, I'll rebase the branch and squash all the O(300) commits together into one and declare you both as co-authors. Hope that's OK? (CC @katilp)

@jmhogan
Copy link
Contributor Author

jmhogan commented Feb 5, 2024

Thanks @tiborsimko that sounds good -- it's too bad that the Github Preview is so helpful for markdown editing compared to rendering the site, very tempting to just hit that "commit" button. We discussed how to avoid so many commits in the future.

@tiborsimko
Copy link
Member

Thanks @tiborsimko that sounds good -- it's too bad that the Github Preview is so helpful for markdown editing compared to rendering the site, very tempting to just hit that "commit" button. We discussed how to avoid so many commits in the future.

Thanks, I'll write over your PR then with everything squashed. BTW no problem in having many small commits whilst developing! It can be very useful. We are simply cleaning branches before merging as the last step, just to get rid of some intermediate commits when they aren't fully necessary to preserve. This facilitates future updates or bug fix hunting. (See also the developing guide.)

This PR adds docs pages for CMS AOD, MiniAOD, and NanoAOD formats,
addressing several tasks in cernopendata#3452.

Some of the contents of these pages refer to records that will be made
in the future. Hopefully all links are currently valid, but several
should updated with new records before new CMS documentation are records
are put into production. (cernopendata#3495, cernopendata#3496, cernopendata#3537, cernopendata#3281, cernopendata#3279)

This slightly addresses cernopendata#3465 by no longer linking to these older
records as examples.

Co-authored-by: Xiaohe Shen <xiaohe_shen@brown.edu>
Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

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

Thanks, squashed as discussed, renamed the location of supporting images, added you and Xiaohe as co-authors.

@tiborsimko tiborsimko merged commit 1556a90 into cernopendata:qa Feb 5, 2024
8 checks passed
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