-
Notifications
You must be signed in to change notification settings - Fork 26
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
Inconsistent sampled values when mixing continuous and discrete distributions #401
Comments
Hi @mlchodkowski, thanks for this bug report. I was aware that this can create issues, and some time ago added a quick hack in the SC sampler such that it would actually return integer values. I'll have a look soon |
@mlchodkowski I'm working in this now, sorry it took so long, was on parental leave due to the birth of my son amongst other things. As a first comment, I think the issue is that you can only have one type in a numpy array. So even if chaospy will give a integer distribution, numpy turns it into a float once combined with the other parameters. A quick workaround that should work for the SC sampler is
The |
@mlchodkowski I think it should be fixed now, without the workaround mentioned above. If you pull the |
@mlchodkowski I went ahead and merged the pull request. You can now check your implementation by just pulling the latest |
Hi @wedeling, first of all, thanks for handling this problem. I've successfully pulled the changes into my local repo. AFAIK the Right now I've encountered a problem with
Additional context (this is my internal JSON format, which is further casted to a proper vary dict):
I've verified that the problem is not in this format, nor the casting-to-vary step. |
I've reopened the issue. Are you using a sparse grid or a standard tensor grid? |
I don't use sparse grid (default arg |
It's a bit hard to see from here, could you post your vary dict and the polynomial order you use? |
Sure, here it is:
EDIT: And here is the model that I'm using:
|
@wedeling Hi, I wanted to inform you that I've encountered the same bug also with SC sampler. The vary is the same as above, sampler is SCSampler with polynomial_order=3. I've tested this on you latest commit (0d27a63) to 'dev' branch. Error:
|
OK, will have a look today. |
It seems to work on my end, I get results like this (1st-order Sobol indices): I used the following script, when storing your model as
The
Could you perhaps run this, and pinpoint at what line it goes wrong? |
Hi, I've recreated my virtualenv in which i installed the newest easyvvuq. I've run your code from above and I got this error in
|
@wedeling I have succesfully localized the bug. The original error was caused by wrong type set in my my pydantic's models for my internal json file. I'm really sorry for the trouble. The error is now fixed. The implementation of SC and PCE samplers allowed for passing the boolean parameters as str's ( However the above error (
I suspect the issue is in the results that my model generates some bad or incompatible results. Maybe it's worth to handle this exception? |
@mlchodkowski no worries, glad you got it working. The |
Closing this |
Bug Report
Description:
I encountered a scenario within my use case where I need to sample both floating-point and integer numbers for my model. In the process of defining a
params
dictionary, I've noticed an inconsistency when combining distributions of typecp.Normal
andcp.DiscreteUniform
within thevary
dictionary. Specifically, when utilizing the SC sampler, integer values are being sampled correctly, but they are incorrectly identified as being of typeclass <float>
. This misidentification triggers constraints related to integer types within theparams
dictionary.Furthermore, when employing PCE and MC samplers, I've observed a behavior where the samplers inconsistently sample both float and integer values. This inconsistency persists despite using the
cp.DiscreteUniform
distribution class (returned values like18.996
or24.52345
). The reported values, despite being constrained to integers, are still indicated asclass <float>
. This behavior is inconsistent with the expected behavior of the distribution.The expectation is that when using
cp.DiscreteUniform
distribution, integer values should be sampled consistently and correctly identified as integer types, not misclassified as floating-point types.Steps to Reproduce:
First case
vary
dictionary with example data like this:As you can see, the values are correctly sampled as integers, but returned as
class<float>
. What's interesting is that when settingpolynomial_order=1
, some parameters, despite being declared ascp.Normal
, were also sampled as integers (still returned asclass<float>
) When setting higherpolynomial_order
, values with distribution set ascp.Normal
or other continuous distribution type are more likely to be floating point numbers like18.2334
.Second case
set_sampler()
step.As you can see here the values were once again returned as
class<float>
but this time they were not integers?steamT
is an integer class, but was sampled as205.3738767467413
, the same goes forsteamP
variable andliquidV
. The only varaibles sampled as integers were ones declared asfloat
.Environment:
Operating System: [e.g.,
Ubuntu 22.04
- WSL1]Python:
3.10.7
EasyVVUQ: currently newest on Pypi,
1.2
.Chaospy version:
4.3.2
Venv manager -
pyenv
Additional Context:
Model that I'm using:
My template file
config.template
:Reproducibility:
Related Issues:
If you found any similar or related issues on the repository, provide links here.
The text was updated successfully, but these errors were encountered: