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

Cross cuts #39

Merged
merged 6 commits into from
Aug 27, 2020
Merged

Cross cuts #39

merged 6 commits into from
Aug 27, 2020

Conversation

@mbaudin47
Copy link
Collaborator Author

@vchabri, @efekhari27 : Please review this PR.

@vchabri
Copy link

vchabri commented Jul 15, 2020

Great work! Thanks Michael.
Here are a few comments about each document:

  1. [CrossCutFunction_Demo.ipynb]:
  • For the last cell with the various figures (A, b, C, D, E, F), I don't get the reason why Figs. D and E show contours which are orthogonal to the ones presented here (https://rprepo.readthedocs.io/en/latest/reliability_problems.html#rp33). If the gradient follows a given stepwise direction (e.g., from left to right), why in these plots it is from bottom to top?
  • Nice explanation at the end of the Notebook!!! Just a question: would it be possible to use/create a method like ".getDesignPoint()" in the X-space or in the U-space as a reference point?
  1. [ConditionalDistribution_Theory.ipynb]:
  • The theory reminders are clear and well-explained. However, there are a few typos in the text ("and", "integeger", typo in the equation f_{X|Xc}).
  1. [ConditionalDistribution_Demo.ipynb]:
  • Nice example. Maybe I could help to provide a few more examples (issued from my PhD manuscript or other resources).
  1. [DrawEvent_demo.ipynb]:
  • Really nice. I got the same question as previously. What is missing in the code to draw similar graphs from a reference point taken as the design point P* ?
  1. [RP8_Demo.ipynb]:
  • Amazing!! Question: Is it the white square in the plots (e.g., top left, or top right) the "removed legend" ?
  • For the other sets of plots, I would suggest to add a marker to the reference point as it is not that clear on the plots.

Hope these remarks will help. The work is great!

@mbaudin47
Copy link
Collaborator Author

mbaudin47 commented Jul 17, 2020

  • [CrossCutFunction_Demo.ipynb] There is a bug in the (x1, x3) projection. Either the RP or myself is wrong.

Below is my version:

Screenshot from 2020-08-27 09-54-37

Here is reference RP33:

Screenshot from 2020-08-27 09-50-50

The first difference is the bounds: [-4, 4] in my graph and [-2, 2] in the reference.

If I use the same bounds, here is the result:

Screenshot from 2020-08-27 09-55-12

If we set x2=0, we get g=min(-x1-x3+3*sqrt(3), -x3 + 3). Here is the result from Mathematica:

https://www.wolframalpha.com/input/?i=min%28-x1-x3%2B3*sqrt%283%29%2C-x3%2B3%29

Screenshot from 2020-08-27 09-59-20

Mathematica produces the same output as mine: the reference output is wrong. The detailed mathematical analysis shows that the reference picture is wrong and this is why I created the associated ticket: https://gitlab.com/rozsasarpi/rprepo/-/issues/20

  • Reference point from design point. With this development, it is possible to draw the function in the X-space. In the U-space, it will be possible by composition of the probabilistic transformation and the function itself. Creating the CrossCut object with this composed function will do the job: this would be very nice example indeed. See Fig3.5 page 40 in Chabridon's thesis for an example. I create the issue in Draw the limit state function in the U-space. #44, as this does not prevent the current PR from being accepted.

  • [ConditionalDistribution_Demo.ipynb] Replace X1=mu1 with X1=2.0. FIxed. Replace conditionalRefencePoint with conditionalReferencePoint: fixed. BayesianDistribution is for Theta, not X. TruncatedDistribution would fit. computeConditionalPDF() is limited with respect to one marginal of X, not all. Useful for Shapley. Dedicated implementation for Normal.

  • CrossCutDistribution / drawConditionalPDF has a bug in the XTitle. Fixed.

  • DrawEvent : specialized in 1D e.g. Y=sin(X) where X is gaussian. I created the following ticket for this: The DrawEvent class should draw 1D events #45.

  • DrawEvent / buildCrossCutFunction currently uses the mean of the distribution as the reference point. It may use another reference point: The buildCrossCutFunction of the DrawEvent class may manage an arbitrary reference point #46.

  • DrawEvent / drawSampleCrossCut currently uses the marginal distribution. It may use the conditional distribution instead: The drawSampleCrossCut method of DrawEvent should use the conditional distribution. #47. Indeed, if we plot the CrossCutFunction by setting conditioned variables and a sample with unconditioned distribution, we compare very different objects. This is why the DrawEvent class must take into account conditional distributions.

  • DrawEvent : add an example with a reference point equal to the design point: There is no example of DrawEvent with design point as reference #48.

  • It would be interesting to draw the marginal distribution and ConditionalDistribution. Add the orange hole example. This will be done within the tech. report on kernel smoothing.

  • draw-cross-cuts.ipynb : insist on the fact that there are 3 different samples. Add a new method with a given sample, drawing projections. This would be an extension of drawInputOutputSample: DrawEvent should be able to manage a given sample #49.

@mbaudin47
Copy link
Collaborator Author

This development is not completely finished, but the current version runs fine and provides interesting features. This is why I merge it.

@mbaudin47 mbaudin47 merged commit 347d1df into master Aug 27, 2020
@mbaudin47 mbaudin47 deleted the CrossCuts branch August 27, 2020 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants