sPHENIX discussion of software

[Benjamin Bannier <bbannier AT bnl.gov>]

Hi Mike,

just following along from the sidelines.

There are two topics on the table here: plain code formatting (like 
breaks, paren placement, etc., ...), and coding style (what patterns are 
encouraged and which should be avoided).

clang-format deals just with formatting, similar to e.g., astyle, but 
does a much much better job at it. It properly understands C++ and C 
(even macros), so you'll get awesome, consistent results. Where it has 
problems it can be disabled locally via `// clang-format <off|on>`. Set 
it up for your style onces (e.g., also add a post-commit hook) and never 
think about formatting code again ;) I used clang-format for the last 
couple of years, at it was a load off my mind.

It does not deal with coding style at all though, that's something the 
Google style guide is mostly about. Disclaimer: I currently work in a 
project that mostly follows the Google style guide but for a small delta 
on top. What I like most about the Google style guide is that is really 
consistent, and that there is some tool support to verify some 
conformance aspects (cpplint, clang-tidy). I don't feel strongly about 
all the rules regarding naming, but this will very hard to enforce for 
you w/o a strong review culture anyway (or writing your own clang-tidy 
checkers). What I don't like is that a substantial part of it is 
motivate by Google's specific needs, e.g., exceptions are completely 
verboten which really limits the usefulness of constructors, or that it 
in some ways discourages value semantics. The consequences of these 
decisions can be felt all through the style guide. There surely are 
valid technical reasons for them, but this does limit how one can write 
safe and at the same time idiomatic C++.



Cheers,

Benjamin

On 2015-12-15 19:54, Michael P. McCumber wrote:
> Hello Jin,
>
> > ACTION ITEM [EVERYONE]: think about a good code style. After the
> > holiday, we will start an email discussion of good choice of code
> > style for us. And make a decision in one of the simulation meetings.
> > A good choice could be the “Google style [1]”.
>
> I was watching youtube talks from CppCon recently---I have odd evening
> entertainment choices---and I ran across an advocate for CLANG-FORMAT
> which is a tool that reformats C++ code into a particular, but
> entirely reasonable choice of style. You can even bind the function
> call into emacs and reformat the document on the fly as you go with a
> simple set strokes. This seems like a very handy tool---we should
> consider this as a way to leap-frog the discussion of picking a style
> (mine is the One True Style) and a method of enforcement (Yuck) and
> just provide STYLE REFORMATTING AS A SERVICE. I've been trying it out
> and it does appear work as advertised. Of course, we should do a
> cross-check on run-time correctness for initial reformatting examples.
>
>
> The idea is that by making the conversion easy and automated it will
> grease the wheels and eliminate time-wasting discussions and let users
> focus on critical issues instead. I like this feature most of all.
>
> Mike
>
> --
>
> Michael P. McCumber, PhD
> Los Alamos National Laboratory
> 505-709-8161
>
> On Wed, Nov 25, 2015 at 7:32 AM, pinkenburg <pinkenburg AT bnl.gov>
> wrote:
>
> > Hi Mike,
> >
> > perfectly good reason,. I am trying to remember why we chose it the
> > other way round - I think it was just that the ids came from G4 so
> > secondaries got a positive id.
> >
> > Thanks
> > Chris
> >
> > On 11/24/2015 5:14 PM, Michael P. McCumber wrote:
> >
> > Hello Chris,
> >
> > o   Chris: Why not choose negative IDs for primary?
> >
> > I left the primary ids positive since it was stated these were
> > being used to translate directly back to the HepMC ids. So I had the
> > flexibility to make the secondaries negative and not the primaries.
> > That was my reasoning.
> >
> > Mike
> >
> > --
> >
> > Michael P. McCumber, PhD
> > Los Alamos National Laboratory
> > 505-709-8161 [3]
> >
> > On Tue, Nov 24, 2015 at 2:07 PM, Huang, Jin <jhuang AT bnl.gov> wrote:
> >
> > Also available on agenda page:
> > https://indico.bnl.gov/conferenceDisplay.py?confId=1349 [4]
> >
> >  
> >
> >  
> >
> > SOFTWARE UPDATE:
> >
> >  
> >
> >  
> >
> > A few major update that is going to break backward compatibility:
> >
> > ·       [MERGED] Generic calorimeter geometry description:
> > https://github.com/sPHENIX-Collaboration/coresoftware/pull/85 [5]
> >
> > ·       [APPROVED TO MERGE] timing cut in calorimeter hits:
> > https://github.com/sPHENIX-Collaboration/coresoftware/pull/87 [6]
> >
> > ·       [APPROVED TO MERGE] Storage Revision for
> > PHG4TruthInfoContainer:
> > https://github.com/sPHENIX-Collaboration/coresoftware/pull/86 [7]
> >
> > o   Conclusion: we agree with the new approach. Mike are welcome
> > to merge, if there are no more major updates to be appended to this
> > pull request
> >
> > o   Chris: Why not choose negative IDs for primary?
> >
> > o   Jin: the proposed ID rule (positive for primary, negative for
> > secondary tracks) looks like a reasonable choice which does the job.
> >
> >
> > ·       [TO BE SUBMITTED BY CHRIS] Architecture to
> > store/retrieve calibration data.
> >
> > o   Chris: for adding the database support, encountered major
> > problem with 64-bit code compatibility with database code
> > infrastructure, need more investigating.
> >
> > o   Chris: Plan to send in pull request with XML and ROOT file
> > calibration support. Database part will be postponed until the
> > problem is resolved.
> >
> >  
> >
> > Plan of moving forward:
> >
> > ·       Pro-build: The new build will not be able to read
> > back pre-CDR simulation sample. A pro-build that support read-back
> > older simulation file or comparison with the new result:
> >
> > * GitHub release tag:
> https://github.com/sPHENIX-Collaboration/coresoftware/releases/tag/pro.1-beta.6
> > [8]
> >
> > * To select this build on RCF:
> > source /opt/sphenix/core/bin/sphenix_setup.csh -n pro.2
> >
> > ·       Regenerate the test simulation data sample.
> >
> > * Plan to regenerate 1000-event single particle sample per
> > kinematic/species, save on disk for general tests.
> >
> >  
> >
> > ACTION ITEM[VOLUNTEER NEEDED]: After all the above updates are
> > merged, we will need to run comparison between old/new sample. Chris
> > would Coordinate.
> >
> >  
> >
> >  
> >
> > DOCUMENTATION UPDATE: JIN HUANG
> >
> >  
> >
> > Action item from last meeting, update the wiki documentation pages.
> > Especially:
> >
> > ·       GitHub password-free ssh login:
> > https://wiki.bnl.gov/sPHENIX/index.php/Code_Repository#GitHub_setup
> > [9]
> >
> > ·       Examples in writing analysis module:
> > https://wiki.bnl.gov/sPHENIX/index.php/Example_of_using_DST_nodes
> > [10]
> >
> > ·       Update core software via Pull requests:
> https://wiki.bnl.gov/sPHENIX/index.php/GitHub_Coresoftware_Update_Procedures
> > [11]
> >
> >  
> >
> > Chris/Nils/John L.: will be good to define a code style and use in
> > the example code (so people could just copy/paste and follow same
> > style).
> >
> >  
> >
> > ACTION ITEM [EVERYONE]: think about a good code style. After the
> > holiday, we will start an email discussion of good choice of code
> > style for us. And make a decision in one of the simulation meetings.
> > A good choice could be the “Google style [1]”.
> >
> >  
> >
> >  
> >
> > Abhisek: shall we make the wikipages in open read access?
> >
> > Chris/Xiaochun: there is a common read-only password (please email
> > privately to ask). We should widely distribute it in the coming
> > collaboration meeting. One can ask ITD for an account for write
> > access.
> >
> > Jin/Chris: better distribution with no pass word reading?
> >
> > Xiaochun: is the GitHub code open?
> >
> > Jin/Chris: yes
> >
> > Xiaochun: then many of the software wiki page should follow to be
> > open in read access.
> >
> >  
> >
> >  
> >
> > TEST BEAM SIMULATION - XIAOCHUN HE
> >
> >  
> >
> > Page 5:
> >
> > Craig: how to adjust location of EMCal for vary angle of Hcal
> > tilt/shift choices.
> >
> > Jin: in leading order we probably keep the vertical location and
> > tilt modules according to the Hcal tilt.
> >
> > We probably should use different holding volume for EMCal and Hcal
> > for similar purpose.
> >
> >  
> >
> > Page 10:
> >
> > Jin: Is there overlap between magenta envelop box?
> >
> > Xiaochun: Yes. This is problem too be resolved by tuning the
> > geometry
> >
> > Chris: try assembly volume or use the current Hcal code as baseline?
> >
> >
> >  
> >
> > ACTION ITEM [XIAOCHUN & CHRIS]: cross check the geometry of Hcal
> > setup in simulation, to be consistent with engineering design and no
> > overlap.
> >
> >  
> >
> > Page 11:
> >
> > Craig/Chris: why MIP hit distribution is so wide?
> >
> > Xiaochun/Jin: no energy cut. Probably low energy photons travel by
> > few radiation length. Need further investigation.
> >
> >  
> >
> > Page 14:
> >
> > Xiaochun: display has been slow
> >
> > Jin: please try the “stored open GL display”. Example is sPHENIX
> > default display macro:
> >
> > /vis/open OGLSX
> >
> > Abhisek: also working on a TGeo-based event display, which is nice
> > for check geometry. To be published in software repository.
> >
> >  
> >
> > Comments on plan:
> >
> > ·       We need to make prototype simulation consistent with
> > sPHENIX full detector simulation (in order to test the sPHENIX full
> > det sim), which require
> >
> > * Geometry/tower tagging in G4 hit (Xiaochun: already have that)
> > * Recent implemented details: Birk effects/timing
> > cut/digitization.
> >
> > ·       Add EMCal + trigger detector + possible tracker
> >
> > ·       Help define beam test run plan
> >
> > ·       Exercise analysis in sPHENIX framework
> >
> > ·       Possible quick result for use in the CALO2016
> > conference
> >
> >  
> >
> > ACTION ITEM: We plan to have a focused effort to push this
> > simulation in the coming Hcal workfest at GSU
> >
> >  
> >
> >  
> >
> > ______________________________
> >
> >  
> >
> > Jin HUANG
> >
> >  
> >
> > Brookhaven National Laboratory
> >
> > Physics Department, Bldg 510 C
> >
> > Upton, NY 11973-5000
> >
> >  
> >
> > Office: 631-344-5898 [12]
> >
> > Cell:   757-604-9946 [13]
> >
> > ______________________________
> >
> >  
> >
> > FROM: Huang, Jin
> > SENT: Monday, November 23, 2015 11:06 AM
> > TO: 'sphenix-l AT lists.bnl.gov' <sphenix-l AT lists.bnl.gov>;
> > 'sphenix-software-l AT lists.bnl.gov'
> > <sphenix-software-l AT lists.bnl.gov>
> > SUBJECT: [reminder] sPHENIX simulation meetings, Tue Nov 24th,
> > 1-3PM EST @ 3-192
> >
> >  
> >
> > Dear All,
> >
> >  
> >
> > This is a reminder for our weekly simulation meeting Tue Nov 24th,
> > 1-3PM EST @ 3-192.
> >
> >  
> >
> > Current on the agenda, there are a few major features proposed/added
> > to the repository. We will go through them on the GitHub page (liked
> > to agenda [4]). Then Xiaochun will show many recent progress on
> > simulating the HCal prototype for the Apr 2016 beam test.
> >
> >  
> >
> > The agenda page is at
> > https://indico.bnl.gov/conferenceDisplay.py?confId=1349 [4]    ,
> > for which the common modification password is "1008"
> >
> > •            For residents at BNL, we will meet in the
> > 3-192 conference room.
> >
> > •            To join the Meeting via bluejeans:
> > https://bluejeans.com/468481110 [14]
> >
> > •            To join via Phone:
> >
> > 1) Dial:
> >
> >         +1 408 740 7256 [15]
> >
> >         +1 888 240 2560 [16](US Toll Free)
> >
> >         +1 408 317 9253 [17](Alternate Number)
> >
> >         (see all numbers - [18]http://bluejeans.com/numbers
> > [18])
> >
> > 2) Enter Conference ID: 468481110
> >
> >  
> >
> > Cheers,
> >
> >  
> >
> > Jin
> >
> >  
> >
> > ______________________________
> >
> >  
> >
> > Jin HUANG
> >
> >  
> >
> > Brookhaven National Laboratory
> >
> > Physics Department, Bldg 510 C
> >
> > Upton, NY 11973-5000
> >
> >  
> >
> > Office: 631-344-5898 [12]
> >
> > Cell:   757-604-9946 [13]
> >
> > ______________________________
> > _______________________________________________
> > Sphenix-l mailing list
> > Sphenix-l AT lists.bnl.gov
> > https://lists.bnl.gov/mailman/listinfo/sphenix-l [19]
> >
> > _______________________________________________
> > Sphenix-software-l mailing list
> > Sphenix-software-l AT lists.bnl.gov
> > https://lists.bnl.gov/mailman/listinfo/sphenix-software-l [2]
>
> --
> *************************************************************
>
> Christopher H. Pinkenburg ; pinkenburg AT bnl.gov
>  ; http://www.phenix.bnl.gov/~pinkenbu [20]
>
> Brookhaven National Laboratory ; phone: (631) 344-5692 [21]
> Physics Department Bldg 510 C ; fax: (631) 344-3253 [22]
> Upton, NY 11973-5000
>
> *************************************************************
>
> _______________________________________________
>  Sphenix-software-l mailing list
>  Sphenix-software-l AT lists.bnl.gov
>  https://lists.bnl.gov/mailman/listinfo/sphenix-software-l [2]
>
>
>
> Links:
> ------
> [1] https://google.github.io/styleguide/cppguide.html
> [2] https://lists.bnl.gov/mailman/listinfo/sphenix-software-l
> [3] tel:505-709-8161
> [4] https://indico.bnl.gov/conferenceDisplay.py?confId=1349
> [5] https://github.com/sPHENIX-Collaboration/coresoftware/pull/85
> [6] https://github.com/sPHENIX-Collaboration/coresoftware/pull/87
> [7] https://github.com/sPHENIX-Collaboration/coresoftware/pull/86
> [8]
> https://github.com/sPHENIX-Collaboration/coresoftware/releases/tag/pro.1-beta.6
> [9] https://wiki.bnl.gov/sPHENIX/index.php/Code_Repository#GitHub_setup
> [10] https://wiki.bnl.gov/sPHENIX/index.php/Example_of_using_DST_nodes
> [11]
> https://wiki.bnl.gov/sPHENIX/index.php/GitHub_Coresoftware_Update_Procedures
> [12] tel:631-344-5898
> [13] tel:757-604-9946
> [14] https://bluejeans.com/468481110
> [15] tel:%2B1%20408%20740%207256
> [16] tel:%2B1%20888%20240%202560
> [17] tel:%2B1%20408%20317%209253
> [18] http://bluejeans.com/numbers
> [19] https://lists.bnl.gov/mailman/listinfo/sphenix-l
> [20] http://www.phenix.bnl.gov/~pinkenbu
> [21] tel:%28631%29%20344-5692
> [22] tel:%28631%29%20344-3253
>
> _______________________________________________
> Sphenix-software-l mailing list
> Sphenix-software-l AT lists.bnl.gov
> https://lists.bnl.gov/mailman/listinfo/sphenix-software-l