Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Container.as_dict for case of flatten=True and add_prefix=True #1887

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Apr 21, 2022

This change has the main goal of having
ImageParametersContainer.as_dict(recursive=True, flatten=True, add_prefix=True) produce a dict with the same keys as the column names
in the dl1 parameters table in the hdf5 file.

Before:

{'params_hillas_hillas_intensity': nan,
 'params_hillas_hillas_skewness': nan,
 'params_hillas_hillas_kurtosis': nan,
 'params_hillas_hillas_fov_lon': <Quantity nan deg>,
 'params_hillas_hillas_fov_lat': <Quantity nan deg>,
 'params_hillas_hillas_r': <Quantity nan deg>,
 'params_hillas_hillas_phi': <Quantity nan deg>,
 'params_hillas_hillas_length': <Quantity nan deg>,
 'params_hillas_hillas_length_uncertainty': <Quantity nan deg>,
 'params_hillas_hillas_width': <Quantity nan deg>,
 'params_hillas_hillas_width_uncertainty': <Quantity nan deg>,
 'params_hillas_hillas_psi': <Quantity nan deg>,
 'params_timing_timing_intercept': nan,
 'params_timing_timing_deviation': nan,
 'params_timing_timing_slope': <Quantity nan 1 / deg>,
 'params_leakage_leakage_pixels_width_1': nan,
 'params_leakage_leakage_pixels_width_2': nan,
 'params_leakage_leakage_intensity_width_1': nan,
 'params_leakage_leakage_intensity_width_2': nan,
 'params_concentration_concentration_cog': nan,
 'params_concentration_concentration_core': nan,
 'params_concentration_concentration_pixel': nan,
 'params_morphology_morphology_num_pixels': -1,
 'params_morphology_morphology_num_islands': -1,
 'params_morphology_morphology_num_small_islands': -1,
 'params_morphology_morphology_num_medium_islands': -1,
 'params_morphology_morphology_num_large_islands': -1,
 'params_intensity_statistics_intensity_max': nan,
 'params_intensity_statistics_intensity_min': nan,
 'params_intensity_statistics_intensity_mean': nan,
 'params_intensity_statistics_intensity_std': nan,
 'params_intensity_statistics_intensity_skewness': nan,
 'params_intensity_statistics_intensity_kurtosis': nan,
 'params_peak_time_statistics_peak_time_max': nan,
 'params_peak_time_statistics_peak_time_min': nan,
 'params_peak_time_statistics_peak_time_mean': nan,
 'params_peak_time_statistics_peak_time_std': nan,
 'params_peak_time_statistics_peak_time_skewness': nan,
 'params_peak_time_statistics_peak_time_kurtosis': nan,
 'params_core_core_psi': <Quantity nan deg>}

After:

In [4]: c.as_dict(recursive=True, flatten=True, add_prefix=True)
Out[4]: 
{'hillas_intensity': nan,
 'hillas_skewness': nan,
 'hillas_kurtosis': nan,
 'hillas_fov_lon': <Quantity nan deg>,
 'hillas_fov_lat': <Quantity nan deg>,
 'hillas_r': <Quantity nan deg>,
 'hillas_phi': <Quantity nan deg>,
 'hillas_length': <Quantity nan deg>,
 'hillas_length_uncertainty': <Quantity nan deg>,
 'hillas_width': <Quantity nan deg>,
 'hillas_width_uncertainty': <Quantity nan deg>,
 'hillas_psi': <Quantity nan deg>,
 'timing_intercept': nan,
 'timing_deviation': nan,
 'timing_slope': <Quantity nan 1 / deg>,
 'leakage_pixels_width_1': nan,
 'leakage_pixels_width_2': nan,
 'leakage_intensity_width_1': nan,
 'leakage_intensity_width_2': nan,
 'concentration_cog': nan,
 'concentration_core': nan,
 'concentration_pixel': nan,
 'morphology_num_pixels': -1,
 'morphology_num_islands': -1,
 'morphology_num_small_islands': -1,
 'morphology_num_medium_islands': -1,
 'morphology_num_large_islands': -1,
 'intensity_max': nan,
 'intensity_min': nan,
 'intensity_mean': nan,
 'intensity_std': nan,
 'intensity_skewness': nan,
 'intensity_kurtosis': nan,
 'peak_time_max': nan,
 'peak_time_min': nan,
 'peak_time_mean': nan,
 'peak_time_std': nan,
 'peak_time_skewness': nan,
 'peak_time_kurtosis': nan,
 'core_psi': <Quantity nan deg>}

This change has the main goal of having
`ImageParametersContainer.as_dict(recursive=True, flatten=True,
add_prefix=True)` produce a dict with the same keys as the column names
in the dl1 parameters table in the hdf5 file.
nbiederbeck
nbiederbeck previously approved these changes Apr 21, 2022
@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #1887 (ce67f31) into master (82a8809) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1887   +/-   ##
=======================================
  Coverage   92.05%   92.05%           
=======================================
  Files         189      189           
  Lines       14862    14868    +6     
=======================================
+ Hits        13681    13687    +6     
  Misses       1181     1181           
Impacted Files Coverage Δ
ctapipe/core/container.py 88.46% <100.00%> (ø)
ctapipe/core/tests/test_container.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82a8809...ce67f31. Read the comment docs.

@kosack kosack added the fix label Apr 22, 2022
@maxnoe maxnoe requested a review from kosack April 29, 2022 12:16
@LukasNickel
Copy link
Member

Does this work with multiple algorithms predicting the same quantity (dl2, ml)?
Havent tested it yet, just my first random thought

@LukasNickel
Copy link
Member

is_valid from the reconstructed shower container subcontainers for example

@maxnoe
Copy link
Member Author

maxnoe commented Jul 4, 2022

We should revisit this after #1949 is merged, changing to draft for now

@maxnoe maxnoe marked this pull request as draft July 4, 2022 17:00
@nbiederbeck
Copy link
Contributor

The use-case for this change is still valid: testing with current master and this branch yields the same results as in the PR description. Therefore, I would suggest merging this, as there are no conflicts and already approved.

Additionally, this fixes a failing test I have on the ml branch.

@nbiederbeck nbiederbeck marked this pull request as ready for review August 3, 2022 08:28
@kosack kosack merged commit 4739e13 into master Aug 3, 2022
@kosack kosack deleted the as_dict_improvements branch August 3, 2022 09:09
nbiederbeck pushed a commit that referenced this pull request Aug 3, 2022
…1887)

* Fix Container.as_dict for case of flatten=True and add_prefix=True

This change has the main goal of having
`ImageParametersContainer.as_dict(recursive=True, flatten=True,
add_prefix=True)` produce a dict with the same keys as the column names
in the dl1 parameters table in the hdf5 file.

* Simplify flatten code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants