Conversation
maximilian-gelbrecht
added
the
input-data
| end | ||
|
|
||
| # for static rasters initialize once and then don't update anymore | ||
| function Terrarium.initialize_from_raster!(field, raster, idxmap, timedim::Nothing) |
There was a problem hiding this comment.
No, please don't do this. This is again a leaky abstraction. Just dispatch internally within the extension module and remove the method from the main package.
There was a problem hiding this comment.
maybe this is a Claude thing... I don't know.
There was a problem hiding this comment.
This is just mirroring the existing update_from_raster! that was defined below already before, just the Terrarium. is too much. Will remove those.
src/timesteppers/model_integrator.jl
Outdated
| # set inputs based on updated clock/state | ||
| # first initialize them, then set them to the current time | ||
| initialize_inputs!(integrator.state, integrator.inputs) | ||
| update_inputs!(integrator.state, integrator.inputs) |
There was a problem hiding this comment.
I think this can be removed.
There was a problem hiding this comment.
The update_inputs!?
Not how it's currently defined, e.g. for the FieldTimeSeriesInput initialize! does just nothing, and you need the update_inputs! that works as it did previously.
There was a problem hiding this comment.
hmm, maybe FieldTimeSeriesInput could just invoke update_inputs! by default?
There was a problem hiding this comment.
Then you also need the clock as an input for initialize!, but I guess that's not a big problem.
There was a problem hiding this comment.
yeah I guess that makes sense actually, could be necessary in some cases.
There was a problem hiding this comment.
ok, done.
|
@maximilian-gelbrecht are the changes done? should I review this again? |
|
Yes. |
2 participants
Resolves #86 and also adds unit tests for the Rasters extension and a convenience wrapper to also directly initialise an
InputSourcefromRingGrids.Fields