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

Update HERON Submodule #2248

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Conversation

dylanjm
Copy link
Collaborator

@dylanjm dylanjm commented Jan 24, 2024


Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

#1114

What are the significant changes in functionality due to this change request?
  • Fix Failing HERON Tests
  • New Economic UQ Features in HERON
  • Dispatch Optimization Refactor

For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

  • 1. Review all computer code.
  • 2. If any changes occur to the input syntax, there must be an accompanying change to the user manual and xsd schema. If the input syntax change deprecates existing input files, a conversion script needs to be added (see Conversion Scripts).
  • 3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
  • 4. Automated Tests should pass, including run_tests, pylint, manual building and xsd tests. If there are changes to Simulation.py or JobHandler.py the qsub tests must pass.
  • 5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test. If new development on the internal JobHandler parallel system is performed, a cluster test must be added setting, in XML block, the node <internalParallel> to True.
  • 6. If the change modifies or adds a requirement or a requirement based test case, the Change Control Board's Chair or designee also needs to approve the change. The requirements and the requirements test shall be in sync.
  • 7. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.
  • 8. If an analytic test is changed/added is the the analytic documentation updated/added?
  • 9. If any test used as a basis for documentation examples (currently found in raven/tests/framework/user_guide and raven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.

@dylanjm dylanjm changed the title Update HERON Submodule [WIP] Update HERON Submodule Jan 24, 2024
@dylanjm dylanjm changed the title [WIP] Update HERON Submodule Update HERON Submodule Jan 24, 2024
@dylanjm
Copy link
Collaborator Author

dylanjm commented Jan 24, 2024

@joshua-cogliati-inl @j-bryan I'm stumped as to why the RAVEN tests are failing on a PR that only has a submodule change. Is there a possibility the failing tests have to do with the recent Distribution changes?

@joshua-cogliati-inl
Copy link
Contributor

@joshua-cogliati-inl @j-bryan I'm stumped as to why the RAVEN tests are failing on a PR that only has a submodule change. Is there a possibility the failing tests have to do with the recent Distribution changes?

I am wondering if there is a problem with a package. As in here is code that causes this error:

Python 3.10.13 | packaged by conda-forge | (main, Dec 23 2023, 15:36:39) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy as np
>>> import scipy.stats
>>> mean = np.zeros(1)
>>> cov=np.eye(1)
>>> mean
array([0.])
>>> cov
array([[1.]])
>>> scipy.stats.multivariate_normal(mean=mean, cov=cov)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/fred/.conda/envs/raven_libraries_new/lib/python3.10/site-packages/scipy/stats/_multivariate.py", line 382, in __call__
    return multivariate_normal_frozen(mean, cov,
  File "/home/fred/.conda/envs/raven_libraries_new/lib/python3.10/site-packages/scipy/stats/_multivariate.py", line 759, in __init__
    self.cov_info = _PSD(self.cov, allow_singular=allow_singular)
  File "/home/fred/.conda/envs/raven_libraries_new/lib/python3.10/site-packages/scipy/stats/_multivariate.py", line 157, in __init__
    s, u = scipy.linalg.eigh(M, lower=lower, check_finite=check_finite)
  File "/home/fred/.conda/envs/raven_libraries_new/lib/python3.10/site-packages/scipy/linalg/_decomp.py", line 547, in eigh
    w, v, *other_args, info = drv(a=a1, **drv_args, **lwork_args)
_flapack.error: (liwork>=max(1,10*n)||liwork==-1) failed for 10th keyword liwork: dsyevr:liwork=1

and I noticed that we went from liblapack 3.9.0 20_linux64_openblas to liblapack 3.9.0 21_linux64_openblas

@PaulTalbot-INL
Copy link
Collaborator

@wangcj05 this looks like an issue with the PCA-reduced multivariate normal not having a symmetric positive semi-definite input matrix. This seems to be a recent failure due to a library change. Did you have to wrestle with this SPD matrix input while setting up this problem?

@j-bryan
Copy link
Collaborator

j-bryan commented Jan 24, 2024

@PaulTalbot-INL There's a try/except block at that part of DistributionsND.py that would catch that ValueError that's initially thrown due to that covariance matrix not being SPD (a planned thing for some covariance matrices that don't play nicely with the scipy.stats.multivariate_normal implementation). I think @joshua-cogliati-inl is on to something; the issue seems to be deeper down in scipy's interaction with LAPACK.

@PaulTalbot-INL
Copy link
Collaborator

Ah, yes, Joshua noted in internal chat:
"""
The error is caught, but then the code to deal with it is failing:

  except ValueError:
      self._distribution = scipy.stats.multivariate_normal(mean=np.zeros(self._rank), cov=np.eye(self._rank))
      self._needsTransform = True

because scipy/lapack are not handling a mean=0, cov=[[1]] distribution.
"""

@wangcj05
Copy link
Collaborator

@wangcj05 this looks like an issue with the PCA-reduced multivariate normal not having a symmetric positive semi-definite input matrix. This seems to be a recent failure due to a library change. Did you have to wrestle with this SPD matrix input while setting up this problem?

@PaulTalbot-INL In the previous process, we do SVD decomposition first, and then using single variate normal distribution for each latent variable, in this case, we do not require the COV to be SPD. But in general, if you sample it directly from MVN, I think a SPD COV is required.

@joshua-cogliati-inl
Copy link
Contributor

This is the pull request that updated lapack in conda: conda-forge/blas-feedstock#113

@joshua-cogliati-inl
Copy link
Contributor

From the other pull request larsoner said :
If you update to the latest SciPy build on conda-forge (following conda-forge/scipy-feedstock#268) or downgrade to OpenBLAS 0.3.25 you should be okay I think

@joshua-cogliati-inl
Copy link
Contributor

So I am guessing that the new version should show up in https://anaconda.org/conda-forge/liblapack/files real soon now?

@j-bryan
Copy link
Collaborator

j-bryan commented Jan 24, 2024

I hope so. Even the internal multivariate_normal object in scipy is failing. Seems to affect only 1-dimensional instantiations of the class, and the default is a 1-dimensional N(0, 1) distribution.

@joshua-cogliati-inl
Copy link
Contributor

Testing if a newer version of scipy will fix this: #2250

@moosebuild
Copy link

Job Mingw Test on 108be4a : invalidated by @PaulTalbot-INL

lib update, rerun tests

@PaulTalbot-INL
Copy link
Collaborator

Checklists pass.

@PaulTalbot-INL PaulTalbot-INL merged commit 83035d3 into idaholab:devel Jan 29, 2024
12 checks passed
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.

6 participants