Implement stats calculations for new calculated_stats_ container
#468
Conversation
src/HealthGPS/analysis_module.h
Outdated
| std::vector<std::string> channels_; | ||
| std::unordered_map<std::string, int> channel_index_; |
There was a problem hiding this comment.
I might not be completely understanding what these data structures do... But am I right in thinking channel_index_ contains indexes into channels_? If so, could you just make channels_ a std::map<std::string, std::string> (I'm guessing it needs to be ordered...) and drop channels_index_?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Add test for calculate_index function
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| auto ses_module = build_ses_noise_module(repository, config); |
There was a problem hiding this comment.
warning: no matching function for call to 'build_ses_noise_module' [clang-diagnostic-error]
auto ses_module = build_ses_noise_module(repository, config);
^Additional context
src/HealthGPS/ses_noise_module.h:44: candidate function not viable: no known conversion from 'std::shared_ptrhgps::ModelInput' to 'const ModelInput' for 2nd argument
std::unique_ptr<SESNoiseModule> build_ses_noise_module(Repository &repository,
^
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
==========================================
+ Coverage 48.47% 51.21% +2.73%
==========================================
Files 164 165 +1
Lines 8284 8370 +86
Branches 1065 1070 +5
==========================================
+ Hits 4016 4287 +271
+ Misses 4259 4074 -185
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
@jamesturner246 @alexdewar I'm going to close this PR without merging. The reason is that this body of work is at a point where it's not possible to make iterative progress to the new way of calculating population statistics (essentially by this I mean running the The feedback I have received in this PR has been useful and has been implemented in the I'm aware that our engagement with HealthGPS has essentially ended, but I really think it's worth finishing this since so much work has gone into it and it's quite close to completion now - it shouldn't take a huge amount of effort to get this finished. |
This PR adds much of the functionality to calculate the stats specified in
channels_. A lot of the logic for this is copied from the existing code, but modified to use thecalculated_stats_vector in place of the existingseriesobject. As a reminder, this is necessary because the calculated stats are being stored in a flat vector (calculated_stats_) which represents a multidimensional array of factors that the user wants to group by, as opposed to theseriesobject. A result of this is the introduction of thecalculate_indexmethod, and theget_channel_indexmethod - both necessary to find the correct index of the factor bin, and "channel" stat in thecalculated_stats_vector, respectively.