kruparell-data-assimilation [3/4]: Add precipitation forcing data assimilation mode with MultiMet support - #286
Conversation
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
In precipitation mode, a tensor-valued x_d_hindcast/x_d silently performs no assimilation: precip_keys stays empty, precip_opts is empty, and the no-key ValueError is only raised when the input is a dict. validate_data_structure() still allows tensor x_d, so this path is reachable. Please reject it explicitly or support tensor forcing inputs.
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
assimilation_lead_time needs an upper-bound check too. If it exceeds seq_length, _end_timestep becomes negative, but the current validation only rejects _end_timestep > seq_length; the run then proceeds with negative assimilation boundaries/slices. Could this require 0 <= assimilation_lead_time <= seq_length?
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
_get_var_lr() uses substring membership to infer state aliases: any target containing c falls back to c_n, and anything containing h falls back to h_n. A target such as precipitation can therefore silently inherit the c_n learning rate when it has no exact entry. Could this map only the documented state aliases rather than single letters?
2 participants
No description provided.