Update schism.data to correctly handle DataBlob #159
Conversation
|
Hi @pbranson This looks reasonable to me, but I might defer here to @benjaminleighton . He is the one that implemented the datablob options here. |
There was a problem hiding this comment.
Hi @pbranson , had a few comments on datablobs vs data objects here
| if isinstance(data, DataBlob): | ||
| anydatablobs = True | ||
| ret[variable] = data.get(destdir, name='sflux') | ||
| existing_nml = ret[variable] / 'sflux_inputs.txt' |
There was a problem hiding this comment.
Line 276, a datablob data.get(destdir, name='sflux') returns either a file or a directory based on data.source but on Line 277, ret[variable] is expected to be a directory in that it is used to define existing_nml as an sflux_inputs.txt in that directory location. That may not be a problem if datablob is always a directory in this case
There was a problem hiding this comment.
Yeah there has to be an sflux_inputs.txt which describes to some extent what the sflux netcdf files contain. If the files are already generated it is assumed this file.
| ret[variable] = data.get(dd, grid, time) | ||
| namelistargs.update(data.namelist) | ||
| if anydatablobs: | ||
| ret["nml"] = existing_nml |
There was a problem hiding this comment.
And with line 276 and 277 this also means you can't mix and match datablobs with other data objects like say some sfluxair datablobs and with sfluxrad data objects. That might be fine but it would be nice to inform the user somehow they need either an all or nothing approach with using datablobs in sflux
There was a problem hiding this comment.
Alternatively I wonder whether ret["nml"] = Sflux_Inputs(**namelistargs).write_nml(destdir) would work even if some of the inputs are datablobs?
There was a problem hiding this comment.
Yeah this is a bit poor, but without interrogating the sflux_input.txt file you dont know how to populate https://github.com/rom-py/rompy/blob/main/rompy/schism/namelists/sflux.py#L8
i.e. **namelistargs is empty for DataBlobs. We could write a parser, but that isnt something done elsewhere to parse model specific input files back into memory.
@tomdurrant I am not totally happy with how I have implemented this but thought it worth getting your feedback on.
The reason being is that currently this won't well handle the case where one of the sflux variables is a datablob but the other isnt. In some respects using datablob only really makes sense for the complete sflux folder already being prepared. This can often be the case if reusing sflux across multiple simulations.
Appreciate your thoughts on this.
cc @benjaminleighton who might have some thoughts here too.