[Reporting][DataFiltering] Expand data padding to full simulation range#2867
[Reporting][DataFiltering] Expand data padding to full simulation range#2867
Conversation
|
Current Coverage: 99% Mypy errors on expand-data-fix-2 branch: 1213 |
|
Current Coverage: 99% Mypy errors on expand-data-fix-2 branch: 1213 |
|
Current Coverage: 99% Mypy errors on expand-data-fix-2 branch: 1213 |
| filtered_simulation_days = sorted(set(all_simulation_days)) | ||
|
|
||
| first_day = filtered_simulation_days[0] if expand_data_to_observed_range else 1 | ||
| last_day = filtered_simulation_days[-1] if expand_data_to_observed_range else simulation_length |
There was a problem hiding this comment.
| last_day = filtered_simulation_days[-1] if expand_data_to_observed_range else simulation_length | |
| last_day = filtered_simulation_days[-1] if expand_data_to_observed_range else simulation_length - 1 |
There was a problem hiding this comment.
Also, I think the RuFaSTime.simulation_length_days is incorrectly calculated, it should be self.simulation_length_days: int = (self.end_date - self.start_date).days + 1 instead.
This simulation_length_days is also used in RuFaSTime.convert_slice_to_simulation_day() function, which gets used in GraphGenerator._draw_graph() to determine the slice start and end. Please also evaluate if we need to update line 165 in RuFaSTime.convert_slice_to_simulation_day() function to remove the extra "+1"
There was a problem hiding this comment.
Thank you, Allister. I think after our discussion at the WT this morning that this is correct so I will make these changes. Good catch!
|
|
||
| first_known_value, first_known_info_map = indexed_data[first_day_of_original_data] | ||
| last_known_value = fill_value | ||
| last_known_info_map = {"simulation_day": 0, "units": original_units} |
There was a problem hiding this comment.
If this last_known_info_map gets used before it gets overwritten, we may not have other information in info_map (like "prefix"). Please look into this!
There was a problem hiding this comment.
ok, I added a fix for this where the original info_map is preserved similar to how it was in the previous version of the function.
|
Current Coverage: 99% Mypy errors on expand-data-fix-2 branch: 1213 |
|
Current Coverage: 99% Mypy errors on expand-data-fix-2 branch: 1213 |
allisterakun
left a comment
There was a problem hiding this comment.
LGTM! Thank you Niko! Great improvements!
New default behavior for data padding is to pad data across the entire simulation length.
Context
Issue(s) closed by this pull request: closes #2725
What
use_fill_value_before_start.Why
Repeated confusion from users on data not being padded as expected if the reporting frequency is not daily.
How
Utilityexpand_data_temporally()is to now pad the data for the full length of the simulation. This will allow any data reported to more easily match the dataset length of the variables reported daily.expand_data_to_observed_rangefilter option set totrue.Test plan
I tried making an exhaustive filter to see the behavior:
{ "multiple": [ { "name": "Expand to Observed (Info Maps) Range - no fill value", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "expand_data_to_observed_range": true, "use_fill_value_before_start": false, "use_fill_value_in_gaps": false, "use_fill_value_at_end": false }, { "name": "Expand full simulation - fill everywhere", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "fill_value": "FILL", "use_fill_value_before_start": true, "use_fill_value_in_gaps": true, "use_fill_value_at_end": true }, { "name": "Expand full simulation - fill everywhere - no fill value provided", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "use_fill_value_before_start": true, "use_fill_value_in_gaps": true, "use_fill_value_at_end": true }, { "name": "Expand full simulation - use carried values everywhere", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "use_fill_value_before_start": false, "use_fill_value_in_gaps": false, "use_fill_value_at_end": false }, { "name": "Expand full simulation - fill start only", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "fill_value": "FILL", "use_fill_value_before_start": true, "use_fill_value_in_gaps": false, "use_fill_value_at_end": false }, { "name": "Expand full simulation - fill gaps only", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "fill_value": "FILL", "use_fill_value_before_start": false, "use_fill_value_in_gaps": true, "use_fill_value_at_end": false }, { "name": "Expand full simulation - fill end only", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ], "expand_data": true, "fill_value": "FILL", "use_fill_value_before_start": false, "use_fill_value_in_gaps": false, "use_fill_value_at_end": true }, { "name": "No expansion", "filters": [ "Feed.Silage.Bunker.corn_silage_storage_1.total_dry_matter_mass" ] } ] }Input Changes
Output Changes
Filter