coveralls
commented
1 year ago Pull Request Test Coverage Report for Build 120a1677-fb8a-4b7d-89b0-72a2c64bd6e9
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 166 unchanged lines in 1 file lost coverage.
- Overall coverage remained the same at 75.672%
| Files with Coverage Reduction | New Missed Lines | % | ||
|---|---|---|---|---|
| /home/circleci/trident/trident/absorption_spectrum/absorption_spectrum_fit.py | 166 | 16.43% | ||
| <!-- | Total: | 166 | --> |
| Totals | |
|---|---|
| Change from base Build 6dcb6d69-c336-4434-aa65-ec6cd87c1122: | 0.0% |
| Covered Lines: | 1745 |
| Relevant Lines: | 2306 |
When fitting very complex spectra, such as with impact parameters < 10 kpc, some students of mine would occasionally have significant components that would not be detected. We eventually decided it was because we had "run out of components" and after some digging we found that this was because there was a maximum of 8 components per complex.
While normally this has no serious impact, and generally using more than 8 components is running against the error limit of the fitter, it was frustrating to not be able to explicitly see and mess with this parameter. I added an optional argument
maxNumCompstogenerate_total_fitto allow this to be changed by the user.Here's some examples below, showing this does come into play with different numbers. 8 is a perfectly good default, but I would prefer if it wasn't hard-coded.
In looking into the source code and comparing with the documentation, I found what might be a more important problem also. The docs say https://trident.readthedocs.io/en/latest/absorption_spectrum_fit.html
However, in line 355, instead of breaking
if np.size(linesP)*3>=len(x):it said to breakif np.size(linesP)+3>=len(x):Which I think was just a typo.
PS: I would use the normal style guide and name this new argument
max_num_comps, but the other arguments in this file are in camelcase so I followed that. Maybe the whole file needs to be restyled?