Video of Ice Concentration Prediction and Ground Truth #61
Conversation
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
| if self.plot_sea_ice_concentration: | ||
| np_ground_truth, np_prediction = _extract_static_data(outputs) | ||
| images = { | ||
| name: plot_fn(np_ground_truth, np_prediction, start_date) | ||
| for name, plot_fn in self.plot_fns.items() | ||
| } |
There was a problem hiding this comment.
Since we only add things to self.plot_fns when they need to be plotted, I think we can skip the if self.plot_sea_ice_concentration check on l.83.
| if self.video_sea_ice_concentration: | ||
| ground_truth_stream, prediction_stream = _extract_video_data(outputs) | ||
| n_timesteps = ground_truth_stream.shape[0] | ||
| sequence_dates = _generate_sequence_dates( | ||
| dataset, batch_idx, batch_size, n_timesteps | ||
| ) | ||
|
|
||
| videos = _create_video_plots( | ||
| ground_truth_stream, | ||
| prediction_stream, | ||
| sequence_dates, | ||
| fps=self.video_fps, | ||
| format=self.video_format, | ||
| ) |
There was a problem hiding this comment.
As above, I think this can be handled by keeping track of a list of functions that we want to apply and applying them each in turn.
| # ----- Static Image Plots ----- | ||
| images = {} | ||
| if self.plot_sea_ice_concentration: | ||
| np_ground_truth, np_prediction = _extract_static_data(outputs) |
There was a problem hiding this comment.
Separating out the extraction code is a great idea :)
| Extract the static data from the outputs. | ||
| """ | ||
|
|
||
| # [batch=0, time=selected_timestep, channels=0, height=:, width=:] |
There was a problem hiding this comment.
This explanation probably lives better in the function docstring (where it will turn up in e.g. IDE tooltips for the function).
| # Load the ground truth and prediction | ||
| np_ground_truth = outputs["target"].cpu().numpy()[0, 0, 0, :, :] | ||
| np_prediction = outputs["output"].cpu().numpy()[0, 0, 0, :, :] | ||
| start_date = dataset.date_from_index(batch_size * batch_idx) |
There was a problem hiding this comment.
Could/should this use _generate_sequence_dates?
| videos["sea-ice-comparison-video"] = video_sic_comparison( | ||
| ground_truth_stream, prediction_stream, dates, fps=fps, format=format | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
It's generally a bad idea to catch Exception (see e.g. https://docs.astral.sh/ruff/rules/blind-except/). Can we check what exceptions are likely to be raised and just catch those?
| for key, video_bytes in videos.items(): | ||
| if hasattr(lightning_logger, "experiment"): # wandb-specific | ||
| try: | ||
| lightning_logger.experiment.log( |
There was a problem hiding this comment.
Is there not a log_video command? Or is it better to use this?
| video_fps: int = 2, | ||
| video_format: Literal["mp4", "gif"] = "mp4", |
There was a problem hiding this comment.
It's good that we know how to change these, but I'm not sure it's useful to make them __init__ arguments (and therefore configurable in the YAML file). Can you think of a reason why we'd want these to be different for different types of model?
Adding ability to make an mp4 or gif animation of the ground truth and predicted sea ice concentration. Exported into wandb in the same way as the images in plotting callbacks. Made some changes to callbacks splitting into helper functions.