Changes to the iterator interface #255
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,100 +1,158 @@ | ||||||
| """ | ||||||
| abstract type AbstractNetworkIterator | ||||||
| A stateful iterator with two states: `increment!` and `compute!`. Each iteration begins | ||||||
| with a call to `increment!` before executing `compute!`, however the initial call to | ||||||
| `iterate` skips the `increment!` call as it is assumed the iterator is initalized such that | ||||||
| this call is implict. Termination of the iterator is controlled by the function `done`. | ||||||
| """ | ||||||
| abstract type AbstractNetworkIterator end | ||||||
|
|
||||||
| # We use greater than or equals here as we increment the state at the start of the iteration | ||||||
| done(NI::AbstractNetworkIterator) = state(NI) >= length(NI) | ||||||
|
|
||||||
| function Base.iterate(NI::AbstractNetworkIterator, init=true) | ||||||
| done(NI) && return nothing | ||||||
| # We seperate increment! from step! and demand that any AbstractNetworkIterator *must* | ||||||
| # define a method for increment! This way we avoid cases where one may wish to nest | ||||||
| # calls to different step! methods accidentaly incrementing multiple times. | ||||||
| init || increment!(NI) | ||||||
| rv = compute!(NI) | ||||||
| return rv, false | ||||||
| end | ||||||
|
|
||||||
| function increment! end | ||||||
| compute!(NI::AbstractNetworkIterator) = NI | ||||||
|
|
||||||
| step!(NI::AbstractNetworkIterator) = step!(identity, NI) | ||||||
| function step!(f, NI::AbstractNetworkIterator) | ||||||
| compute!(NI) | ||||||
| f(NI) | ||||||
| increment!(NI) | ||||||
| return NI | ||||||
| end | ||||||
|
|
||||||
| # | ||||||
| # RegionIterator | ||||||
| # | ||||||
| """ | ||||||
| struct RegionIterator{Problem, RegionPlan} <: AbstractNetworkIterator | ||||||
| """ | ||||||
| mutable struct RegionIterator{Problem,RegionPlan} <: AbstractNetworkIterator | ||||||
| problem::Problem | ||||||
| region_plan::RegionPlan | ||||||
| const sweep::Int | ||||||
| which_region::Int | ||||||
| function RegionIterator(problem::P, region_plan::R, sweep::Int) where {P,R} | ||||||
| return new{P,R}(problem, region_plan, sweep, 1) | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| state(R::RegionIterator) = R.which_region | ||||||
| Base.length(R::RegionIterator) = length(R.region_plan) | ||||||
|
|
||||||
| problem(R::RegionIterator) = R.problem | ||||||
|
|
||||||
| current_region_plan(R::RegionIterator) = R.region_plan[R.which_region] | ||||||
|
|
||||||
| function current_region(R::RegionIterator) | ||||||
| region, _ = current_region_plan(R) | ||||||
| return region | ||||||
| end | ||||||
|
|
||||||
| function current_region_kwargs(R::RegionIterator) | ||||||
| _, kwargs = current_region_plan(R) | ||||||
| return kwargs | ||||||
| end | ||||||
|
|
||||||
| function previous_region(R::RegionIterator) | ||||||
| state(R) <= 1 && return nothing | ||||||
| prev, _ = R.region_plan[R.which_region - 1] | ||||||
| return prev | ||||||
| end | ||||||
|
|
||||||
| function next_region(R::RegionIterator) | ||||||
| is_last_region(R) && return nothing | ||||||
| next, _ = R.region_plan[R.which_region + 1] | ||||||
| return next | ||||||
| end | ||||||
| is_last_region(R::RegionIterator) = length(R) === state(R) | ||||||
|
|
||||||
| # | ||||||
| # Functions associated with RegionIterator | ||||||
| # | ||||||
|
|
||||||
| function compute!(R::RegionIterator) | ||||||
| region_kwargs = current_region_kwargs(R) | ||||||
| R.problem = region_step(R; region_kwargs...) | ||||||
| return R | ||||||
| end | ||||||
| function increment!(R::RegionIterator) | ||||||
| R.which_region += 1 | ||||||
| return R | ||||||
| end | ||||||
|
|
||||||
| function RegionIterator(problem; sweep, sweep_kwargs...) | ||||||
| plan = region_plan(problem; sweep, sweep_kwargs...) | ||||||
| return RegionIterator(problem, plan, sweep) | ||||||
| end | ||||||
|
|
||||||
| function region_step( | ||||||
| region_iterator; extract_kwargs=(;), update_kwargs=(;), insert_kwargs=(;), kws... | ||||||
| ) | ||||||
| prob = problem(region_iterator) | ||||||
|
|
||||||
| sweep = region_iterator.sweep | ||||||
|
|
||||||
| prob, local_state = extract(prob, region_iterator; extract_kwargs..., sweep, kws...) | ||||||
| prob, local_state = update(prob, local_state, region_iterator; update_kwargs..., kws...) | ||||||
| prob = insert(prob, local_state, region_iterator; sweep, insert_kwargs..., kws...) | ||||||
| return prob | ||||||
| end | ||||||
|
|
||||||
| function region_plan(problem; kws...) | ||||||
| return euler_sweep(state(problem); kws...) | ||||||
| end | ||||||
|
|
||||||
| # | ||||||
| # SweepIterator | ||||||
| # | ||||||
|
|
||||||
| mutable struct SweepIterator{Problem} <: AbstractNetworkIterator | ||||||
| sweep_kws | ||||||
|
||||||
| region_iter::RegionIterator{Problem} | ||||||
| which_sweep::Int | ||||||
| function SweepIterator(problem, sweep_kws) | ||||||
| sweep_kws = Iterators.Stateful(sweep_kws) | ||||||
|
||||||
| sweep_kws = Iterators.Stateful(sweep_kws) | |
| stateful_sweep_kws = Iterators.Stateful(sweep_kws) |
I would find that a bit clearer (i.e. it is easier to keep track that we are now using the stateful version of sweep_kws).
Outdated
There was a problem hiding this comment.
Related to our discussion around how the RegionIterator gets updated and how the problem gets passed around, maybe it could help to wrap this constructor call into a function, for example:
function update_region_iterator(iterator::RegionIterator; kwargs...)
return RegionIterator(iterator.problem; kwargs...)
endor something like that (mostly to hide the detail of how the problem gets passed along at this level of the code).
There was a problem hiding this comment.
Also, what does SR stand for? Generally I prefer lower case variable names, and also more descriptive ones, say sweep_iter.
There was a problem hiding this comment.
I think SR was an abbreviation of SweepRegionIterator which was a previous name for something I was using previously...
On the first point, I feel like update_region_iterator is not really descriptive of what the function as implemented there does. It does not update an existing RegionIterator, it creates a new one via the constructor. I do like however the following:
function update_region_iterator!(iterator::SweepIterator; kwargs...)
iterator.region_iter = new_region_iterator(iterator.region_iter; kwargs...)
return iterator
endwhere new_region_iterator is a renaming of the method you describe. Let me know if you agree.
There was a problem hiding this comment.
It does not update an existing
RegionIterator, it creates a new one via the constructor. I do like however the following:
I suppose that's a matter of perspective, i.e. we could think of it as a fancy kind of "setter" method that keeps some properties and sets other ones, which for immutable types has to create a new object by definition, but I'm ok with new_region_iterator.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small recommendation, I would prefer a variable name besides
NC, maybe we could just useiterator.