Remove const and mutable since they're not required.

[vkuznet]

As far as I can tell from code, there is no need to use const/mutable within a class for selected methods.

[vkuznet]

@Dr15Jones please review

[cmsbuild]

A new Pull Request was created by @vkuznet (Valentin Kuznetsov) for CMSSW_7_0_X.

Remove const and mutable since they're not required.

It involves the following packages:

FastSimulation/TrackingRecHitProducer

@cmsbuild, @nclopezo, @giamman, @lveldere can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.

[Dr15Jones]

Unfortunately, those are virtual functions from the base class ClusterParameterEstimator

http://cmslxr.fnal.gov/lxr/source/RecoLocalTracker/ClusterParameterEstimator/interface/ClusterParameterEstimator.h#012

So the fix would be to remove const from the base class and change all inheriting classes.

[vkuznet]

I wanted to know if it is appropriate approach in this case. If I modify base class, then SiClusterTranslator.cc will not compile, see its produce method
http://cmslxr.fnal.gov/lxr/source/FastSimulation/TrackingRecHitProducer/src/SiClusterTranslator.cc#078

But I'm not sure that the produce method logic is correct in context of EventSetup. It extracts StripCPE/FastCPE objects from EventSetup, clear up their internal maps and refill it again.
Could you please review it and let me know your thoughts on this.

On Nov 7, 2013, at ,Nov 7, 8:54 AM, Chris Jones wrote:

Unfortunately, those are virtual functions from the base class ClusterParameterEstimator

http://cmslxr.fnal.gov/lxr/source/RecoLocalTracker/ClusterParameterEstimator/interface/ClusterParameterEstimator.h#012

So the fix would be to remove const from the base class and change all inheriting classes.

—
Reply to this email directly or view it on GitHub.

[Dr15Jones]

It looks to me like the design of the base class ClusterParameterEstimator directly conflicts with CMS' rules about data products not being allowed to change.
My suggestion is we change all the modifying methods to be 'non-const' and add a 'clone()' method which returns a std::unique_ptr<ClusterParameterEstimator>. Classes which inherit from ClusterParameterEstimator have to implement the 'clone()' method. Then users can get the 'ClusterParameterEstimator' from the EventSetup but they call 'clone' to get their own copy which can be modified.

[ktf]

@vkuznet this does not merge anymore. Can you update it?

[cmsbuild]

Pull request #1369 was updated. @nclopezo, @lveldere, @cmsbuild, @thspeer, @giamman, @slava77 can you please check and sign again.

[ktf]

mmm... it still does not merge in the last IB...

[vkuznet]

is it possible to see merge problem somehow? I can try to re-apply changes again within fresh branch.

[vkuznet]

Giulio, I regenerated all changes in separate branch, I think it would be easier if we close this one (without merging of course) and I submit new pull request from another branch. Possibly I didn't rebase and it cause merging problem in this branch. Here is new branch with the same set of changes: https://github.com/vkuznet/cmssw/compare/ThreadSafe2_FastSimulation_TrackingRecHitProducer?expand=1