Commit 7896cd6c authored by Jorn Bruggeman's avatar Jorn Bruggeman
Browse files

more informative circular dependency tracking/reporting

parent 3bebadaa
...@@ -44,7 +44,6 @@ module fabm_graph ...@@ -44,7 +44,6 @@ module fabm_graph
type (type_node_list_member), pointer :: last => null() type (type_node_list_member), pointer :: last => null()
contains contains
procedure :: append => node_list_append procedure :: append => node_list_append
procedure :: pop => node_list_pop
procedure :: remove => node_list_remove procedure :: remove => node_list_remove
procedure :: finalize => node_list_finalize procedure :: finalize => node_list_finalize
procedure :: find => node_list_find procedure :: find => node_list_find
...@@ -52,6 +51,13 @@ module fabm_graph ...@@ -52,6 +51,13 @@ module fabm_graph
procedure :: check => node_list_check procedure :: check => node_list_check
end type end type
type type_call_stack_node
class (type_base_model), pointer :: model => null()
integer :: source = source_unknown
type (type_internal_variable), pointer :: requested_variable => null()
type (type_call_stack_node), pointer :: previous => null()
end type
type type_output_variable type type_output_variable
type (type_internal_variable), pointer :: target => null() type (type_internal_variable), pointer :: target => null()
type (type_node_set) :: dependent_nodes type (type_node_set) :: dependent_nodes
...@@ -288,14 +294,14 @@ recursive function graph_has_descendant(self, graph) result(has_descendant) ...@@ -288,14 +294,14 @@ recursive function graph_has_descendant(self, graph) result(has_descendant)
has_descendant = .false. has_descendant = .false.
end function end function
recursive function graph_add_call(self, model, source, outer_calls) result(node) recursive function graph_add_call(self, model, source, stack_top) result(node)
class (type_graph), target,intent(inout) :: self class (type_graph), target, intent(inout) :: self
class (type_base_model),target,intent(in) :: model class (type_base_model), target, intent(in) :: model
integer, intent(in) :: source integer, intent(in) :: source
type (type_node_list), target,intent(inout) :: outer_calls type (type_call_stack_node), pointer :: stack_top
type (type_node), pointer :: node type (type_node), pointer :: node
type (type_node_list_member), pointer :: pnode type (type_call_stack_node), pointer :: existing_stack_node, stack_node
character(len=2048) :: chain character(len=2048) :: chain
class (type_graph), pointer :: root_graph, owner_graph, target_graph class (type_graph), pointer :: root_graph, owner_graph, target_graph
integer :: operation integer :: operation
...@@ -305,16 +311,22 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node) ...@@ -305,16 +311,22 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node)
! Circular dependency check: ! Circular dependency check:
! Search the list of outer model calls, i.e., calls that [indirectly] request the current call. ! Search the list of outer model calls, i.e., calls that [indirectly] request the current call.
! If the current call is already on this list, it is indirectly calling itself: there is a circular dependency. ! If the current call is already on this list, it is indirectly calling itself: there is a circular dependency.
pnode => outer_calls%find_node(model, source) existing_stack_node => stack_top
if (associated(pnode)) then do while (associated(existing_stack_node))
if (associated(existing_stack_node%model, model) .and. existing_stack_node%source == source) exit
existing_stack_node => existing_stack_node%previous
end do
if (associated(existing_stack_node)) then
! Circular dependency found - report as fatal error. ! Circular dependency found - report as fatal error.
chain = '' chain = trim(model%get_path()) // ':' // trim(source2string(source))
do while (associated(pnode)) stack_node => stack_top
chain = trim(chain) // ' ' // trim(pnode%p%as_string()) // ' ->' do
pnode => pnode%next chain = trim(stack_node%model%get_path()) // ':' // trim(source2string(stack_node%source)) &
// ' needs ' // trim(stack_node%requested_variable%name) // ' provided by ' // trim(chain)
if (associated(stack_node, existing_stack_node)) exit
stack_node => stack_node%previous
end do end do
call driver%fatal_error('graph::add_call', 'circular dependency found: ' // trim(chain(2:)) & call driver%fatal_error('graph::add_call', 'circular dependency found: ' // trim(chain))
// ' ' // trim(model%get_path()) // ':' // trim(source2string(source)))
end if end if
! By default we add the call to the current graph (but if necessary we will target an ancestor instead) ! By default we add the call to the current graph (but if necessary we will target an ancestor instead)
...@@ -359,7 +371,10 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node) ...@@ -359,7 +371,10 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node)
! First add this call to the list of requesting calls [a list of all calls higher on the call stack] ! First add this call to the list of requesting calls [a list of all calls higher on the call stack]
! This forbids any indirect dependency on this call, as such would be a circular dependency. ! This forbids any indirect dependency on this call, as such would be a circular dependency.
call outer_calls%append(node) allocate(stack_node)
stack_node%model => model
stack_node%source = source
stack_node%previous => stack_top
link => model%links%first link => model%links%first
do while (associated(link)) do while (associated(link))
...@@ -367,21 +382,22 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node) ...@@ -367,21 +382,22 @@ recursive function graph_add_call(self, model, source, outer_calls) result(node)
! This is the model's own variable (not inherited from child model) and the model itself originally requested read access to it. ! This is the model's own variable (not inherited from child model) and the model itself originally requested read access to it.
_ASSERT_(.not. associated(link%target%write_owner), 'graph::add_call', 'BUG: required input variable is co-written.') _ASSERT_(.not. associated(link%target%write_owner), 'graph::add_call', 'BUG: required input variable is co-written.')
input_variable => node%inputs%add(link%target) input_variable => node%inputs%add(link%target)
stack_node%requested_variable => link%target
if (.not. (associated(link%target%owner, model) .and. link%target%source == source)) & if (.not. (associated(link%target%owner, model) .and. link%target%source == source)) &
call target_graph%add_variable(link%target, outer_calls, input_variable%sources, caller=node) call target_graph%add_variable(link%target, stack_node, input_variable%sources, caller=node)
end if end if
link => link%next link => link%next
end do end do
! Remove node from the list of outer calls and add it to the graph instead. ! Remove node from the list of outer calls and add it to the graph instead.
node => outer_calls%pop() deallocate(stack_node)
call target_graph%append(node) call target_graph%append(node)
end function graph_add_call end function graph_add_call
recursive subroutine graph_add_variable(self, variable, outer_calls, variable_set, copy_to_store, caller) recursive subroutine graph_add_variable(self, variable, stack_top, variable_set, copy_to_store, caller)
class (type_graph), intent(inout) :: self class (type_graph), intent(inout) :: self
type (type_internal_variable), intent(in) :: variable type (type_internal_variable), intent(in) :: variable
type (type_node_list), target, intent(inout) :: outer_calls type (type_call_stack_node), pointer :: stack_top
type (type_output_variable_set), intent(inout) :: variable_set type (type_output_variable_set), intent(inout) :: variable_set
logical, optional, intent(in) :: copy_to_store logical, optional, intent(in) :: copy_to_store
type (type_node), optional, target, intent(inout) :: caller type (type_node), optional, target, intent(inout) :: caller
...@@ -411,7 +427,7 @@ contains ...@@ -411,7 +427,7 @@ contains
if (variable%source == source_constant .or. variable%source == source_state .or. variable%source == source_external .or. variable%source == source_unknown) return if (variable%source == source_constant .or. variable%source == source_state .or. variable%source == source_external .or. variable%source == source_unknown) return
_ASSERT_ (.not. variable%write_indices%is_empty(), 'graph_add_variable::add_call', 'Variable "' // trim(variable%name) // '" with source ' // trim(source2string(variable%source)) // ' does not have a write index') _ASSERT_ (.not. variable%write_indices%is_empty(), 'graph_add_variable::add_call', 'Variable "' // trim(variable%name) // '" with source ' // trim(source2string(variable%source)) // ' does not have a write index')
node => self%add_call(variable%owner, variable%source, outer_calls) node => self%add_call(variable%owner, variable%source, stack_top)
output_variable => node%outputs%add(variable) output_variable => node%outputs%add(variable)
if (present(copy_to_store)) output_variable%copy_to_store = output_variable%copy_to_store .or. copy_to_store if (present(copy_to_store)) output_variable%copy_to_store = output_variable%copy_to_store .or. copy_to_store
if (present(caller)) then if (present(caller)) then
...@@ -546,29 +562,6 @@ subroutine node_list_check(self) ...@@ -546,29 +562,6 @@ subroutine node_list_check(self)
end do end do
end subroutine node_list_check end subroutine node_list_check
function node_list_pop(self) result(node)
class (type_node_list), intent(inout) :: self
type (type_node),pointer :: node
type (type_node_list_member), pointer :: previous
node => null()
if (associated(self%last)) then
node => self%last%p
previous => self%last%previous
if (associated(previous)) then
! More than one node in list.
previous%next => null()
else
! Pop-ed node is only one in list.
self%first => null()
end if
deallocate(self%last)
self%last => previous
end if
!call self%check()
end function node_list_pop
subroutine node_list_remove(self, node) subroutine node_list_remove(self, node)
class (type_node_list), intent(inout) :: self class (type_node_list), intent(inout) :: self
type (type_node), target :: node type (type_node), target :: node
......
...@@ -683,7 +683,6 @@ contains ...@@ -683,7 +683,6 @@ contains
type (type_global_variable_register), intent(inout) :: variable_register type (type_global_variable_register), intent(inout) :: variable_register
type (type_job_node), pointer :: job_node type (type_job_node), pointer :: job_node
type (type_node_list) :: outer_calls
type (type_variable_request),pointer :: variable_request type (type_variable_request),pointer :: variable_request
type (type_call_request), pointer :: call_request, next_call_request type (type_call_request), pointer :: call_request, next_call_request
type (type_node), pointer :: graph_node type (type_node), pointer :: graph_node
...@@ -703,7 +702,7 @@ contains ...@@ -703,7 +702,7 @@ contains
! We clean up [deallocate] the variable and call requests at the same time. ! We clean up [deallocate] the variable and call requests at the same time.
variable_request => self%first_variable_request variable_request => self%first_variable_request
do while (associated(variable_request)) do while (associated(variable_request))
call self%graph%add_variable(variable_request%variable, outer_calls, variable_request%output_variable_set, copy_to_store=variable_request%store) call self%graph%add_variable(variable_request%variable, null(), variable_request%output_variable_set, copy_to_store=variable_request%store)
if (variable_request%store) then if (variable_request%store) then
! FABM must be able to provide data for this variable across the entire spatial domain. ! FABM must be able to provide data for this variable across the entire spatial domain.
! If this variable is a constant, explicitly request a data field for it in the persistent store. ! If this variable is a constant, explicitly request a data field for it in the persistent store.
...@@ -719,12 +718,11 @@ contains ...@@ -719,12 +718,11 @@ contains
call_request => self%first_call_request call_request => self%first_call_request
do while (associated(call_request)) do while (associated(call_request))
next_call_request => call_request%next next_call_request => call_request%next
graph_node => self%graph%add_call(call_request%model, call_request%source, outer_calls) graph_node => self%graph%add_call(call_request%model, call_request%source, null())
deallocate(call_request) deallocate(call_request)
call_request => next_call_request call_request => next_call_request
end do end do
self%first_call_request => null() self%first_call_request => null()
call outer_calls%finalize()
!self%graph%frozen = .true. !self%graph%frozen = .true.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment