add phi0 to chord_length

[burmist-git]

Add phi0 to chord_length
related to :
#2845

[mexanick]

LGTM. Coverage is underreported because of numba.

[burmist-git]

@maxnoe These are tests - they are good. Could you please review this long lasting question.

[maxnoe]

See inline comments.

[maxnoe]

These are tests - they are good.

These are not tests. It's a collection of plots for which I have no idea how they were produced.

[maxnoe]

There is a code change here for a bugfix, but no new test making sure the bug is actually fixed.

So independent of the numba coverage underreporting, there is indeed missing coverage here.

Please add a test that fails on the current main and passes after the changes you make here to demonstrate what this fixes and prevent changes from breaking it again.

[maxnoe]

There is also no test added for the new functionality of passing in phi0 as an argument.

And I have to ask again: what is the motivation for this change? Why do you want to push the rotation business into these lower level function compared to the muon image prediction?

[maxnoe]

Ok, please rebase now that #2873 is merged.

[burmist-git]

@maxnoe @mexanick please merge.

[maxnoe]

You could already use thee new parameter now in the intensity fit likelihood.

[burmist-git]

I prefer to first add "my" new lightweight method for this. This new method is more efficient and, more importantly, it provides an entry point for ML algorithms.

[ctao-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[burmist-git]

great !!!