-
Notifications
You must be signed in to change notification settings - Fork 133
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/ft generator #1370
base: devel
Are you sure you want to change the base?
Mandd/ft generator #1370
Conversation
This PR requires a new python library: SymPy (https://www.sympy.org/en/index.html) |
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.
Several comments for you. @mandd
@@ -30,6 +30,7 @@ \subsection{PostProcessor} | |||
\item \textbf{ParetoFrontier} | |||
\item \textbf{MCSImporter} | |||
\item \textbf{EconomicRatio} | |||
\item \textbf{FTgenerator} |
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.
I suggest to change FTgenerator
to FTGenerator
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.
If you agree, please go over all the files to update it.
contained in the PointSet. This postprocessor creates a fault tree based on the generated Boolean formula. | ||
|
||
\ppType{SampleSelector}{SampleSelector} |
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.
update SampleSelector to FTGenerator
\begin{enumerate} | ||
\item the input variables in the PointSet which correspond to the basic events in the fault tree | ||
\item the output variable in the PointSet which correspond to the top event in the fault tree |
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.
correspond --> corresponds?
\item \xmlNode{topEventID}, \xmlDesc{string, required field}, ID of the output variable in the PointSet which corresponds | ||
to the top event in the fault tree | ||
\item \xmlNode{inputVars}, \xmlDesc{string, required field}, IDs of the input variables in the PointSet which correspond |
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.
string --> list of strings (comma separated strings)
from utils import utils, xmlUtils | ||
from utils import InputData, InputTypes | ||
from sympy.logic import SOPform, POSform | ||
from itertools import product |
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.
move following lines to External Modules above
from sympy.logic import SOPform, POSform
from itertools import product
inputVars = paramInput.findFirst('inputVars') | ||
self.inputVars = inputVars.value.split(",") | ||
for var in self.inputVars: | ||
var = var.strip() |
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.
No need to use strip method here, it is already in InputTypes.
self.simOnly = simOnly.value | ||
|
||
typeConv = paramInput.findFirst('type') | ||
self.type = typeConv.value.lower() |
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.
maybe worth to add lower option in InputTypes, and a discussion with our teams. No need to change here.
understandable by this pp. | ||
In this case, we only want data objects! | ||
@ In, currentInp, list, an object that needs to be converted | ||
@ Out, currentInp, DataObject.HistorySet, input data |
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.
Change DataObject.HistorySet to DataObject.PointSet, only PointSet is allowed.
inData = self.inputToInternal(inputIn) | ||
dataset = inData.asDataset() | ||
|
||
self.dataObjectName = inputIn |
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.
self variable should be first mentioned in init method.
for counter,combination in enumerate(combinations): | ||
indexes = np.where(np.all(data==combination,axis=1))[0] | ||
if len(indexes)==0 and not self.simOnly: | ||
self.raiseAnError(RuntimeError,'FTgenerator: combination ' + str(combination) + ' of variables ' + str(self.inputVars) +' has not been found in the dataset.') |
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.
'+' can be replaced with ','
@mandd You may want to finalize this PR. In addition, I think PR belongs to SR2ML. Please let me know what do you think. |
@mandd Should we move this to SR2ML and finalize the PR? |
Pull Request Description
What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)
Creation of a PP which generates fault tree from a set of simulated data (PointSet) #1369
What are the significant changes in functionality due to this change request?
For Change Control Board: Change Request Review
The following review must be completed by an authorized member of the Change Control Board.
<internalParallel>
to True.raven/tests/framework/user_guide
andraven/docs/workshop
) have been changed, the associated documentation must be reviewed and assured the text matches the example.