Request branch2 #22
Request branch2 #22
Conversation
|
Please translate the comment into english so as to make the repo as open as possible. Please gather the unnecessary commits created for Circle CI into a single one with "squash" with |
circleci circleci2 circleci3 circleci4 circleci5 circleci6
requested BBRC correction notebook circleci issue notebook circleci issue second try minor changes
efekhari27
force-pushed
the
request_branch2
branch
from
6862254
to
c438f64
Compare
|
Hi Michaël ! Here is the remote ReliabilityProblem functionality allowing us to build a otbenchmark ReliabilityProblem using limit state functions from the BBRC server. The useless commit have been squashed as requested. I saw that you are working on a way to build the BBRC distribution table without pandas, i'm sure it can improve this development. Best |
notebook metadata environnement name
efekhari27
force-pushed
the
request_branch2
branch
from
c2195fc
to
d8469db
Compare
You can reproduce it with your own Spyder with Control + I. Please insert a blank line before each section, e.g. before "Parameters".
That should be the last step before integration. |
examples/RequestedBBRCProblem.ipynb
Outdated
| "source": [ | ||
| "# Test de la classe RequestedBBRCProblem\n", | ||
| "\n", | ||
| "L'objectif de cet exemple est de présenter l'utilisation de la classe RequestedBBRCProblem" |
There was a problem hiding this comment.
Please translate it into english.
| "algoProb = ot.ProbabilitySimulationAlgorithm(event)\n", | ||
| "algoProb.setMaximumOuterSampling(10)\n", | ||
| "algoProb.setMaximumCoefficientOfVariation(0.01)\n", | ||
| "algoProb.run()" |
There was a problem hiding this comment.
Please insert a comment that explains that your intent in selecting a low value of the maximumOuterSampling : it is chosen to make a fast example, but a more realistic parameter value (e.g. 1000) should be chosen in practice. This will clarify your motivations in the example. Please insert the code to print the estimated probability, because it is the concrete goal of the example but add a comment that explains that, because of the low value of the maximumOuterSampling, the estimate is not very accurate.
requirements.txt
Outdated
| @@ -2,6 +2,7 @@ numpy==1.16.* | |||
| matplotlib==2.* | |||
| scipy==1.* | |||
| openturns>=1.14 | |||
| pandas | |||
There was a problem hiding this comment.
The pandas dependency is not required anymore. Please remove it.
tests/test_ReliabilityProblem110.py
Outdated
| @@ -13,22 +13,21 @@ | |||
|
|
|||
There was a problem hiding this comment.
Fixing the style of the unit tests is good. However, I guess that a separate PR would have made this review clearer. In this PR, it hides the fact that there is no unit test for the new classes, letting the reader think that there are unit tests, while there are not. Please avoid this for future PRs. Anyway, keep them as is for this one to smooth the process.
|
I you need help to finalize the PR: I would be a pleasure to fix it, although I think that you would better finalize it for yourself. |
efekhari27
force-pushed
the
request_branch2
branch
from
e0a7adc
to
f0d2011
Compare
Add distribution file in otbenchmark rep. Add distribution file in otbenchmark rep. 2 Add distribution file in otbenchmark rep. 3
efekhari27
force-pushed
the
request_branch2
branch
from
f0d2011
to
9be744f
Compare
|
@efekhari27: Please solve the conflict: this seems to be the very last required step for integration. |
|
|
||
| types = ["i4", "i4", "i4", "i4", "U15", "f8", "f8", "f8", "f8", "f8", "f8"] | ||
| bbrc_dist_table = np.genfromtxt( | ||
| "otbenchmark/distributions/probabilistic_models.csv", |
There was a problem hiding this comment.
This line is problematic, because the csv file may not be available on the user's side. This makes the CI fail. Moreover, the file may not be installed on user's computer after setup.py or installed when the module is installed with pip.
|
I will get this PR on a new branch and try to make so that it gets in the package. |
Here is the remote ReliabilityProblem functionality allowing us to build a otbenchmark ReliabilityProblem using limit state functions from the BBRC server. The useless commit have been squashed as requested. I saw that you are working on a way to build the BBRC distribution table without pandas, i'm sure it can improve this development.
@vchabri @JEBROUN pour information :)