-
Notifications
You must be signed in to change notification settings - Fork 8
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
Mandd/multipleknapsack models #17
Changes from 10 commits
ef636b9
251920c
9356a64
1ae2668
c37bbc6
d56539c
c65556e
fe314ac
eee6250
72c5a32
6141ab1
b4dd50f
cb3b3b3
465135e
639ba75
5e01fe3
b072c64
f6ab6a5
9710ed2
d4b0b12
e0c2572
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,12 +9,13 @@ \section{Knapsack Models} | |
find the optimal solution. | ||
|
||
|
||
\subsection{BaseKnapsackModel} | ||
\subsection{BaseKnapsack Model} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BaseKnapsackModel --> SimpleKnapsackModel? If so, please update this section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, fixed all section |
||
\label{subsec:BaseKnapsackModel} | ||
This model considers the classical Knapsack Model characterized by a set of elements | ||
that can be chosen (or not). | ||
The goal is to maximize the sum of the chosen element values provided that the sum of | ||
element cost values satisfy capacity constraint (specified in the \xmlNode{capacity} node). | ||
element cost values satisfy capacity constraint (specified in the variable defined | ||
in the \xmlNode{capacity} node). | ||
|
||
The ID of the variables that represent cost, value, and choice of each element are | ||
indicated in the \xmlNode{map} node. | ||
|
@@ -30,21 +31,69 @@ \subsection{BaseKnapsackModel} | |
is not satisfied, then the \xmlNode{choiceValue} variable is penalized by multiplying the | ||
project value by -\xmlNode{penaltyFactor}. | ||
|
||
Example LOGOS input XML for DRO: | ||
Example LOGOS input XML for BaseKnapsack Model: | ||
\begin{lstlisting}[style=XML] | ||
<Models> | ||
<ExternalModel name="knapsack" subType="LOGOS.BaseKnapsackModel"> | ||
<variables>element1Status,element2Status,element3Status, | ||
element1Val ,element2Val ,element3Val, | ||
element1Cost ,element2Cost ,element3Cost, | ||
validity ,totalValue </variables> | ||
<capacity>10</capacity> | ||
<variables>element1Status,element2Status,element3Status,element4Status,element5Status, | ||
element1Val ,element2Val ,element3Val ,element4Val ,element5Val, | ||
element1Cost ,element2Cost ,element3Cost ,element4Cost,element5Cost, | ||
validity,totalValue,capacityID</variables> | ||
<capacity>capacityID</capacity> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, capacity is required input/variable from RAVEN perspective, and you do not allow user to directly provide this value. Right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is correct, the goal is to have knapsack parameters as raven generated variables |
||
<penaltyFactor>1.</penaltyFactor> | ||
<outcome>validity</outcome> | ||
<choiceValue>totalValue</choiceValue> | ||
<map value='element1Val' cost='element1Cost' >element1Status</map> | ||
<map value='element2Val' cost='element2Cost' >element2Status</map> | ||
<map value='element3Val' cost='element3Cost' >element3Status</map> | ||
<map value='element4Val' cost='element4Cost' >element4Status</map> | ||
<map value='element5Val' cost='element5Cost' >element5Status</map> | ||
</ExternalModel> | ||
\end{lstlisting} | ||
|
||
|
||
\subsection{MultipleKnapsack Model} | ||
\label{subsec:MultipleKnapsackModel} | ||
This model considers the Multiple Knapsack Model characterized by a set of elements | ||
that can be chosen (or not) over a set of multiple knapsacks. | ||
The goal is to maximize the sum of the chosen element values provided that the sum of | ||
element cost values satisfy capacity constraints of each knapsack. | ||
|
||
The capacity of each knapsack is defined in the \xmlNode{knapsack} node. | ||
|
||
The ID of the variables that represent cost, value, and choice of each element are | ||
indicated in the \xmlNode{map} node. | ||
The model generates two variables: | ||
\begin{itemize} | ||
\item the validity of the chosen solution (specified in the \xmlNode{outcome} node): either | ||
valid (i.e., 0), or invalid (i.e., 1) if the capacity constraint is not satisfied, | ||
\item totalValue (specified in the \xmlNode{choiceValue} node): sum of the values of the | ||
chosen elements | ||
\end{itemize} | ||
|
||
When calculating the \xmlNode{choiceValue} variable, if the capacity constraints | ||
are not satisfied, then the \xmlNode{choiceValue} variable is penalized by multiplying the | ||
project value by -\xmlNode{penaltyFactor}. | ||
|
||
Example LOGOS input XML for MultipleKnapsack Model: | ||
\begin{lstlisting}[style=XML] | ||
<Models> | ||
<ExternalModel name="knapsack" subType="LOGOS.MultipleKnapsackModel"> | ||
<variables>e1Status,e2Status,e3Status,e4Status,e5Status, | ||
e1Val ,e2Val ,e3Val ,e4Val ,e5Val, | ||
e1Cost ,e2Cost ,e3Cost ,e4Cost ,e5Cost, | ||
validity,totalValue, | ||
K1_cap,K2_cap,K3_cap</variables> | ||
<knapsack ID='1'>K1_cap</knapsack> | ||
<knapsack ID='2'>K2_cap</knapsack> | ||
<knapsack ID='3'>K3_cap</knapsack> | ||
<penaltyFactor>1.</penaltyFactor> | ||
<outcome>validity</outcome> | ||
<choiceValue>totalValue</choiceValue> | ||
<map value='e1Val' cost='e1Cost' >e1Status</map> | ||
<map value='e2Val' cost='e2Cost' >e2Status</map> | ||
<map value='e3Val' cost='e3Cost' >e3Status</map> | ||
<map value='e4Val' cost='e4Cost' >e4Status</map> | ||
<map value='e5Val' cost='e5Cost' >e5Status</map> | ||
</ExternalModel> | ||
</Models> | ||
\end{lstlisting} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ class BaseKnapsackModel(ExternalModelPluginBase): | |
""" | ||
This class is designed to create the BaseKnapsack model | ||
""" | ||
|
||
@classmethod | ||
def getInputSpecs(cls): | ||
""" | ||
|
@@ -32,7 +33,7 @@ def getInputSpecs(cls): | |
inputSpecs = InputData.parameterInputFactory('ExternalModel') | ||
inputSpecs.addParam('name', param_type=InputTypes.StringType, required=True) | ||
inputSpecs.addParam('subType', param_type=InputTypes.StringType, required=True) | ||
inputSpecs.addSub(InputData.parameterInputFactory('capacity', contentType=InputTypes.FloatType)) | ||
inputSpecs.addSub(InputData.parameterInputFactory('capacity', contentType=InputTypes.StringType)) | ||
inputSpecs.addSub(InputData.parameterInputFactory('penaltyFactor', contentType=InputTypes.FloatType)) | ||
inputSpecs.addSub(InputData.parameterInputFactory('outcome', contentType=InputTypes.StringType)) | ||
inputSpecs.addSub(InputData.parameterInputFactory('choiceValue', contentType=InputTypes.StringType)) | ||
|
@@ -69,6 +70,7 @@ def _readMoreXML(self, container, xmlNode): | |
@ Out, None | ||
""" | ||
container.mapping = {} | ||
|
||
specs = self.getInputSpecs()() | ||
specs.parseNode(xmlNode) | ||
for node in specs.subparts: | ||
|
@@ -108,16 +110,19 @@ def run(self, container, inputDict): | |
@ In, inputDict, dict, dictionary of inputs from RAVEN | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can make the run class as abstractmethod There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
totalValue = 0.0 | ||
capacity = inputDict[self.capacity][0] | ||
print(capacity) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove print? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
|
||
for key in container.mapping: | ||
if key in inputDict.keys() and inputDict[key] in [0.0,1.0]: | ||
if inputDict[key] == 1.0: | ||
testValue = self.capacity - inputDict[container.mapping[key][1]] | ||
testValue = capacity - inputDict[container.mapping[key][1]] | ||
if testValue >= 0: | ||
self.capacity = self.capacity - inputDict[container.mapping[key][1]] | ||
capacity = capacity - inputDict[container.mapping[key][1]] | ||
totalValue = totalValue + inputDict[container.mapping[key][0]] | ||
else: | ||
self.capacity = self.capacity - inputDict[container.mapping[key][1]] | ||
print('======') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove print? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
capacity = capacity - inputDict[container.mapping[key][1]] | ||
totalValue = totalValue - inputDict[container.mapping[key][0]] * self.penaltyFactor | ||
elif inputDict[key] == 0.0: | ||
pass | ||
|
@@ -126,7 +131,7 @@ def run(self, container, inputDict): | |
else: | ||
raise IOError("BaseKnapsackModel: variable " + str(key) + " is not found in the set of input variables.") | ||
|
||
if self.capacity>=0: | ||
if capacity>=0: | ||
container.__dict__[self.outcome] = 0. | ||
else: | ||
container.__dict__[self.outcome] = 1. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
# Copyright 2020, Battelle Energy Alliance, LLC | ||
# ALL RIGHTS RESERVED | ||
""" | ||
Created on February 7, 2021 | ||
|
||
@author: mandd | ||
""" | ||
|
||
#External Modules--------------------------------------------------------------- | ||
import numpy as np | ||
import math | ||
import copy | ||
#External Modules End----------------------------------------------------------- | ||
|
||
#Internal Modules--------------------------------------------------------------- | ||
from PluginsBaseClasses.ExternalModelPluginBase import ExternalModelPluginBase | ||
from utils import InputData, InputTypes | ||
#Internal Modules End----------------------------------------------------------- | ||
|
||
|
||
class MultipleKnapsackModel(ExternalModelPluginBase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you inherit directly from BaseKnapsackModel? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a subset of elements that are in common between these two classes. |
||
""" | ||
This class is designed to create the MultipleKnapsack model | ||
""" | ||
|
||
@classmethod | ||
def getInputSpecs(cls): | ||
""" | ||
Collects input specifications for this class. | ||
@ In, None | ||
@ Out, inputSpecs, InputData, input specifications | ||
""" | ||
inputSpecs = InputData.parameterInputFactory('ExternalModel') | ||
inputSpecs.addParam('name' , param_type=InputTypes.StringType, required=True) | ||
inputSpecs.addParam('subType', param_type=InputTypes.StringType, required=True) | ||
|
||
inputSpecs.addSub(InputData.parameterInputFactory('penaltyFactor', contentType=InputTypes.FloatType)) | ||
inputSpecs.addSub(InputData.parameterInputFactory('outcome' , contentType=InputTypes.StringType), required=True) | ||
inputSpecs.addSub(InputData.parameterInputFactory('choiceValue' , contentType=InputTypes.StringType), required=True) | ||
|
||
knapsack = InputData.parameterInputFactory('knapsack', contentType=InputTypes.StringType) | ||
knapsack.addParam('ID', param_type=InputTypes.StringType, required=True) | ||
inputSpecs.addSub(knapsack) | ||
|
||
mapping = InputData.parameterInputFactory('map', contentType=InputTypes.StringType) | ||
mapping.addParam('value', param_type=InputTypes.StringType, required=True) | ||
mapping.addParam('cost', param_type=InputTypes.StringType, required=True) | ||
inputSpecs.addSub(mapping) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you inherit directly from BaseKnapsackModel, most of these lines can be removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, having a base class would simplify the structure of getInputSpecs and _readMoreXML methods. |
||
|
||
return inputSpecs | ||
|
||
def __init__(self): | ||
""" | ||
Constructor | ||
@ In, None | ||
@ Out, None | ||
""" | ||
ExternalModelPluginBase.__init__(self) | ||
|
||
self.penaltyFactor = 1.0 # penalty factor that is used when the capacity constraint is not satisfied | ||
self.outcome = None # ID of the variable which indicates if the chosen elements satisfy the capacity constraint | ||
self.choiceValue = None # ID of the variable which indicates the sum of the values of the chosen project elements | ||
|
||
def _readMoreXML(self, container, xmlNode): | ||
""" | ||
Method to read the portion of the XML that belongs to the MultipleKnapsack model | ||
@ In, container, object, self-like object where all the variables can be stored | ||
@ In, xmlNode, xml.etree.ElementTree.Element, XML node that needs to be read | ||
@ Out, None | ||
""" | ||
container.mapping = {} | ||
self.knapsackSet = {} | ||
|
||
for child in xmlNode: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update this with specs = self.getInputSpecs()()
specs.parseNode(xmlNode) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the overall _readMoreXML method |
||
if child.tag == 'penaltyFactor': | ||
self.penaltyFactor = float(child.text.strip()) | ||
elif child.tag == 'outcome': | ||
self.outcome = child.text.strip() | ||
elif child.tag == 'choiceValue': | ||
self.choiceValue = child.text.strip() | ||
elif child.tag == 'knapsack': | ||
self.knapsackSet[child.get('ID')] = child.text.strip() | ||
elif child.tag == 'map': | ||
container.mapping[child.text.strip()] = [child.get('value'),child.get('cost')] | ||
elif child.tag == 'variables': | ||
variables = [str(var.strip()) for var in child.text.split(",")] | ||
else: | ||
raise IOError("MultipleKnapsackModel: xml node " + str (child.tag) + " is not allowed") | ||
|
||
|
||
def initialize(self, container, runInfoDict, inputFiles): | ||
""" | ||
Method to initialize the MultipleKnapsack model | ||
@ In, container, object, self-like object where all the variables can be stored | ||
@ In, runInfoDict, dict, dictionary containing all the RunInfo parameters (XML node <RunInfo>) | ||
@ In, inputFiles, list, list of input files (if any) | ||
@ Out, None | ||
""" | ||
pass | ||
|
||
|
||
def run(self, container, inputDict): | ||
""" | ||
This method calculates the sum of the chosen element values and check if the capacity constraints | ||
for all knapsacks are satisfied | ||
@ In, container, object, self-like object where all the variables can be stored | ||
@ In, inputDict, dict, dictionary of inputs from RAVEN | ||
""" | ||
totalValue = 0.0 | ||
knapsackSetValues={} | ||
|
||
# knapsackSetValues is a dictionary in the form {knapsackID: knapsackValue} | ||
for knapsack in self.knapsackSet.keys(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. check if self.knapsackSet[knapsack] in inputDict first? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. check has been added |
||
knapsackSetValues[knapsack] = inputDict[self.knapsackSet[knapsack]][0] | ||
|
||
# List of allowed knapsack IDs | ||
elementAllowedValues = list(map(float, self.knapsackSet.keys())) | ||
# Add 0.0 which implies that the element has not been assigned to any knapsack | ||
elementAllowedValues.append(0.0) | ||
|
||
for key in container.mapping: | ||
if key in inputDict.keys() and inputDict[key] in elementAllowedValues: | ||
if inputDict[key] > 0.0: | ||
knapsackChosen = str(int(inputDict[key][0])) | ||
testValue = knapsackSetValues[knapsackChosen] - inputDict[container.mapping[key][1]] | ||
if testValue >= 0: | ||
knapsackSetValues[knapsackChosen] = knapsackSetValues[knapsackChosen] - inputDict[container.mapping[key][1]][0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line can be moved to the outside of this if condition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code has been updated, similar issue was located in the base knapsack model which has been edited as well |
||
totalValue = totalValue + inputDict[container.mapping[key][0]] | ||
else: | ||
knapsackSetValues[knapsackChosen] = knapsackSetValues[knapsackChosen] - inputDict[container.mapping[key][1]][0] | ||
totalValue = totalValue - inputDict[container.mapping[key][0]] * self.penaltyFactor | ||
else: | ||
raise IOError("MultipleKnapsackModel: variable " + str(key) + " is either not found in the set of input variables or its values is not allowed.") | ||
|
||
numberUnsatConstraints = 0.0 | ||
for knapsack in knapsackSetValues.keys(): | ||
if knapsackSetValues[knapsack] < 0: | ||
numberUnsatConstraints = numberUnsatConstraints + 1. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This loop can be combined with previous for loop. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point: merged with previous loop |
||
|
||
if numberUnsatConstraints > 0.0 : | ||
container.__dict__[self.outcome] = 1. | ||
else: | ||
container.__dict__[self.outcome] = 0. | ||
|
||
print(knapsackSetValues) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove print? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
container.__dict__[self.choiceValue] = totalValue |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
element1Status,element2Status,element3Status,element4Status,element5Status,element1Val,element2Val,element3Val,element4Val,element5Val,element1Cost,element2Cost,element3Cost,element4Cost,element5Cost,validity,totalValue | ||
1.0,0.0,1.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,0.0,13.0 | ||
1.0,1.0,1.0,0.0,0.0,1,2,3,4,5,1,1,1,1,1,0.0,6.0 | ||
0.0,1.0,1.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,4.0 | ||
1.0,0.0,1.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,-13.0 | ||
0.0,0.0,1.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,-12.0 | ||
1.0,1.0,0.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,-12.0 | ||
1.0,0.0,0.0,0.0,0.0,1,2,3,4,5,1,1,1,1,1,1.0,-1.0 | ||
1.0,1.0,1.0,1.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,-15.0 | ||
0.0,0.0,1.0,1.0,0.0,1,2,3,4,5,1,1,1,1,1,1.0,-7.0 | ||
1.0,1.0,0.0,0.0,1.0,1,2,3,4,5,1,1,1,1,1,1.0,-8.0 | ||
element1Status,element2Status,element3Status,element4Status,element5Status,element1Val,element2Val,element3Val,element4Val,element5Val,element1Cost,element2Cost,element3Cost,element4Cost,element5Cost,capacityID,validity,totalValue | ||
1.0,0.0,1.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,3.0 | ||
1.0,1.0,1.0,0.0,0.0,1,2,3,4,5,2,3,3,4,4,10,0.0,6.0 | ||
0.0,1.0,1.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,4.0 | ||
1.0,0.0,1.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,3.0 | ||
0.0,0.0,1.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,2.0 | ||
1.0,1.0,0.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,2.0 | ||
1.0,0.0,0.0,0.0,0.0,1,2,3,4,5,2,3,3,4,4,10,0.0,1.0 | ||
1.0,1.0,1.0,1.0,1.0,1,2,3,4,5,2,3,3,4,4,10,1.0,-3.0 | ||
0.0,0.0,1.0,1.0,0.0,1,2,3,4,5,2,3,3,4,4,10,0.0,7.0 | ||
1.0,1.0,0.0,0.0,1.0,1,2,3,4,5,2,3,3,4,4,10,0.0,8.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
e1Status,e2Status,e3Status,e4Status,e5Status,e1Val,e2Val,e3Val,e4Val,e5Val,e1Cost,e2Cost,e3Cost,e4Cost,e5Cost,K1_cap,K2_cap,K3_cap,validity,totalValue | ||
2.0,0.0,2.0,3.0,3.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,13.0 | ||
3.0,2.0,2.0,1.0,0.0,1,2,3,4,5,2,3,3,4,4,5,5,5,0.0,10.0 | ||
0.0,2.0,2.0,2.0,2.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,14.0 | ||
3.0,1.0,2.0,2.0,2.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,15.0 | ||
1.0,0.0,3.0,3.0,3.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,13.0 | ||
2.0,3.0,0.0,3.0,2.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,12.0 | ||
2.0,1.0,1.0,0.0,1.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,11.0 | ||
3.0,2.0,2.0,2.0,2.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,15.0 | ||
0.0,1.0,3.0,3.0,1.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,14.0 | ||
2.0,2.0,0.0,1.0,2.0,1,2,3,4,5,2,3,3,4,4,5,5,5,1.0,12.0 |
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.
You do not need to import BaseKnapsackModel since this model can not be directly used by RAVEN.
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.
good point fixed