Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensemble Model Connections Improvement #732

Merged
merged 3 commits into from
Aug 9, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions framework/Models/EnsembleModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,6 @@ def initialize(self,runInfo,inputs,initDict=None):
# construct the ensemble model directed graph
self.ensembleModelGraph = graphStructure.graphObject(modelsToOutputModels)
# make some checks
# FIXME: the following check is too tight, even if the models are connected, the
# code may still raise an error. I think in really, we do not need to raise an error,
# maybe a warning is enough. For example:
# a -> b -> c
# ^
# |
# e -> d -> f
if not self.ensembleModelGraph.isConnectedNet():
isolatedModels = self.ensembleModelGraph.findIsolatedVertices()
self.raiseAnError(IOError, "Some models are not connected. Possible candidates are: "+' '.join(isolatedModels))
Expand Down
4 changes: 2 additions & 2 deletions framework/OutStreams/OutStreamPrint.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ def localGetInitParams(self):
for index in range(len(self.sourceName)):
paramDict['Source Name ' + str(index) + ' :'] = self.sourceName[index]
if self.what:
for index in range(len(self.what)):
paramDict['Variable Name ' + str(index) + ' :'] = self.what[index]
for index, var in enumerate(self.what.split(',')):
paramDict['Variable Name ' + str(index) + ' :'] = var
return paramDict

def initialize(self, inDict):
Expand Down
49 changes: 27 additions & 22 deletions framework/utils/graphStructure.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#External Modules------------------------------------------------------------------------------------
import sys
import itertools
import copy
#External Modules End--------------------------------------------------------------------------------
#Internal Modules------------------------------------------------------------------------------------
from utils import utils
Expand Down Expand Up @@ -127,7 +128,7 @@ def findIsolatedVertices(self):
@ In, None
@ Out, isolated, list, list of isolated nodes (verteces)
"""
graph = self.__graphDict
graph = self.__extendDictForGraphTheory()
isolated = []
for vertex in graph:
if not graph[vertex]:
Expand Down Expand Up @@ -202,52 +203,56 @@ def findAllPaths(self, startVertex, endVertex, path=[]):
paths.append(p)
return paths

def isConnected(self, verticesEncountered = None, startVertex=None):
def isConnected(self, graphDict, verticesEncountered = None, startVertex=None):
"""
Method that determines if the graph is connected (graph theory connectivity)
@ In, verticesEncountered, set, the encountered vertices (generally it is None when called from outside)
@ In, graphDict, dict, the graph dictionary
@ In, startVertex, string, the starting vertex
@ Out, isConnected, bool, is the graph fully connected?
"""
if verticesEncountered is None:
verticesEncountered = set()
gdict = self.__graphDict
vertices = list(gdict.keys())
vertices = list(graphDict.keys())
if not startVertex:
# chosse a vertex from graph as a starting point
startVertex = vertices[0]
verticesEncountered.add(startVertex)
if len(verticesEncountered) != len(vertices):
for vertex in gdict[startVertex]:
for vertex in graphDict[startVertex]:
if vertex not in verticesEncountered:
if self.isConnected(verticesEncountered, vertex):
if self.isConnected(graphDict, verticesEncountered, vertex):
return True
else:
return True
return False

def isConnectedNet(self, startVertex=None):
def isConnectedNet(self):
"""
Method that determines if the graph is a connected net (all the vertices are connect with each other through other vertices)
This method can state that we have a connected net even if, based on graph theory, the graph is not connected
@ In, startVertex, string, the starting vertex
@ In, None
@ Out, graphNetConnected, bool, is the graph net fully connected?
"""
graphNetConnected = self.isConnected()
if graphNetConnected:
return graphNetConnected
# the graph is not connected (Graph theory)
uniquePaths = self.findAllUniquePaths()
# now check if there is at list a node that is common in all the paths
if len(uniquePaths) > 0:
graphNetConnected = True
startPath = set(uniquePaths.pop())
for otherPath in uniquePaths:
if startPath.isdisjoint(otherPath):
graphNetConnected = False
break
graphDict = self.__extendDictForGraphTheory()
graphNetConnected = self.isConnected(graphDict)
return graphNetConnected

def __extendDictForGraphTheory(self):
"""
Method to extend the graph dictionary in order to be accepted by the graph theory
@ In, None
@ Out, graphDict, dict, the extended graph dictionary, used for isConnectedNet and findIsolatedVertices
"""
graphDict = copy.deepcopy(self.__graphDict)
# Extend graph dictionary generated by ensemble model to graph theory acceptable dictionary
# Basicly, if a -> b (a is connected to b), the self.__graphDict = {'a':['b'], 'b':[]}
# Since a is connected to b, from the graph theory, the dictionary should become {'a':['b'], 'b':['a']}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class was supposed to represent a directed graph since was mainly used by the ensemble model for the identification of the execution order.
Please, put a WARNING to avoid to store this extended graph dictionary in __graphDict and specify that this should be used for the isConnected method only.

In addition, enhance the comment above (and in the above docstrings) to state this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

for inModel, outModelList in self.__graphDict.items():
for outModel in outModelList:
if inModel not in graphDict[outModel]:
graphDict[outModel].append(inModel)
return graphDict

def findAllUniquePaths(self, startVertices=None):
"""
This method finds all the unique paths in the graph
Expand Down
28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/alpha.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you do not need the lock here (for this model). Please remove it and remove the comment above. In addition all the above imports are not used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed.


def initialize(self, runInfo, inputs):
pass
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not needed... if it is not present, it will not be called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed.


def run(self, Input):
self.C = self.A + self.B
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add docstrings to describe what this model is used for

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/beta.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()

def initialize(self, runInfo, inputs):
pass
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments I reported for model alpha.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed


def run(self, Input):
self.E = self.C - self.D + 1.0
28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/delta.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()

def initialize(self, runInfo, inputs):
pass

def run(self, Input):
self.F = self.E + 1.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments i reported for the other 2 models

Copy link
Collaborator Author

@wangcj05 wangcj05 Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/epsilon.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()

def initialize(self, runInfo, inputs):
pass

def run(self, Input):
self.G = self.H + 3.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/gamma.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()

def initialize(self, runInfo, inputs):
pass

def run(self, Input):
self.D = self.G + 1.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

28 changes: 28 additions & 0 deletions tests/framework/ensembleModelTests/checkConnection/rho.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright 2017 Battelle Energy Alliance, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import numpy as np
import copy
import time
# CAUTION HERE #
# IT IS IMPORTANT THAT THE USER IS AWARE THAT THE EXTERNAL MODEL (RUN METHOD)
# NEEDS TO BE THREAD-SAFE!!!!!
# IN CASE IT IS NOT, THE USER NEEDS TO SET A LOCK SYSTEM!!!!!
import threading
localLock = threading.RLock()

def initialize(self, runInfo, inputs):
pass

def run(self, Input):
self.I = self.D - 1.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Loading