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

Adding DSS capabilities #1619

Merged
merged 82 commits into from
Dec 6, 2021
Merged

Adding DSS capabilities #1619

merged 82 commits into from
Dec 6, 2021

Conversation

yoshiurr-INL
Copy link
Collaborator

@yoshiurr-INL yoshiurr-INL commented Jul 21, 2021


Pull Request Description

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

Closes #1617

What are the significant changes in functionality due to this change request?
  • The main function of this pull request is to add Dynamical System Scaling (DSS) to wangc/validation_pp_base.
  • The addition of metric and postprocessor DSS code is part of the PR.
  • To pass DSS specific data to metric code DSS, the MetricDistributor.py has been modified. When "DSS" is added as the metric in the xml file, the modified code will pass postprossed data to only the DSS metric code.
  • Test files showing the DSS functionality is added as well.

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.

# assert
assert(isinstance(inputIn, list))
assert(isinstance(inputIn[0], xr.Dataset) or isinstance(inputIn[0], DataObjects.DataSet))
# the input can be either be a list of dataobjects or a list of datasets (xarray)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These asserts will error out!
If they are necessary, maybe assert(isinstance(inputIn['Data][0][-1],xr.Dataset)) ?

Edited due to invalid assert errors.
@moosebuild
Copy link

Job Precheck on 07da9b7 : invalidated by @joshua-cogliati-inl

checking civet.

"""
# assert
assert(isinstance(inputIn["Data"], list))
assert(isinstance(inputIn["Data"][0][-1], xr.Dataset) and isinstance(inputI["Data"][1][-1], xr.Dataset))
Copy link
Collaborator

@Jimmy-INL Jimmy-INL Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a missing n in inputI['Data'][1][-1]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to push it my self but it gave me:
remote: Password authentication is temporarily disabled as part of a brownout. Please use a personal access token instead. remote: Please see https://github.blog/2020-07-30-token-authentication-requirements-for-api-and-git-operations/ for more information. fatal: unable to access 'https://github.com/idaholab/raven.git/': The requested URL returned error: 403

Base automatically changed from wangc/validation_pp_base to devel September 28, 2021 17:08
@wangcj05
Copy link
Collaborator

@yoshiurr-INL Can you rebase your branch with devel and try to resolve the conflicts?

yoshiurr-INL and others added 6 commits October 11, 2021 09:57
As part of last fiscal year's milestone completion, the code was repeatedly tested under various types of time dependent data. The code was improved for each issue encountered. The import for "Validation" has been updated to "ValidationBase".
After rebase, forgot to select which head to change before committing changes to this branch.
Changed line 173 to adapt with new input format implemented after converting to ValidationBase.py.
@wangcj05 wangcj05 added the RAVENv2.2 for RAVENv2.2 Release label Oct 16, 2021
@wangcj05
Copy link
Collaborator

@yoshiurr-INL @Jimmy-INL This branch has been cleaned up.

@wangcj05
Copy link
Collaborator

@yoshiurr-INL Is it ready to be reviewed?

Readjusted the postprocessed data output name.
The output name is changed according to the Probabilistic model in the user manual.
Added more test files
Deleting to test civet errors
Deleting to test civet errors
Deleting to test civet errors
Deleting to test civet errors
Deleting to test civet errors
Deleting to test civet errors
Returning to previous state
@moosebuild
Copy link

Job Test Fedora 31 on cbc7fe1 : invalidated by @wangcj05

retesting

@moosebuild
Copy link

Job Test Fedora 32 on cbc7fe1 : invalidated by @Jimmy-INL

@@ -404,6 +404,7 @@
\input{reducedOrderModeling.tex}
\input{statisticalAnalysis.tex}
\input{dataMining.tex}
\input{dssPostProcessor.tex}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not uploaded to your remote branch.

Copy link
Collaborator

@wangcj05 wangcj05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoshiurr-INL I have some additional comments for your consideration. In addition:

  1. there is a missing .tex file for theory manual
  2. I am wondering if you can desire a analytic test case for DSS in future PR.


The specifications of a DSS metric is defined within the \xmlNode{Metric} XML block. The attribute \xmlAttr{subType} must be \textbf{PPDSS} (see \ref{subsubsec:Validation}) in the \xmlNode{PostProcessor} for the outputs of the post-processor to be in the right format for DSS metric inputs.

This XML node needs to contain the attributes:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems 'attributes' is still used.

doc/user_manual/metrics.tex Outdated Show resolved Hide resolved
framework/Models/PostProcessors/Validations/PPDSS.py Outdated Show resolved Hide resolved
framework/Models/PostProcessors/Validations/PPDSS.py Outdated Show resolved Hide resolved
framework/Models/PostProcessors/Validations/PPDSS.py Outdated Show resolved Hide resolved
framework/Models/PostProcessors/Validations/PPDSS.py Outdated Show resolved Hide resolved
@wangcj05
Copy link
Collaborator

wangcj05 commented Dec 1, 2021

@yoshiurr-INL There are some compiling issue with the theory manual, could you check that?

@moosebuild
Copy link

Job Test qsubs falcon on 9632050 : invalidated by @yoshiurr-INL

@yoshiurr-INL yoshiurr-INL requested a review from wangcj05 December 2, 2021 20:18
Copy link
Collaborator

@wangcj05 wangcj05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is good to me.

@moosebuild
Copy link

Job Mingw Test on f81f692 : invalidated by @joshua-cogliati-inl

restarted civet

@moosebuild
Copy link

Job Mingw Test on f81f692 : invalidated by @yoshiurr-INL

Need to restart checking process due to win-mingw was not restarted.

@yoshiurr-INL yoshiurr-INL merged commit 9d8db47 into devel Dec 6, 2021
@yoshiurr-INL yoshiurr-INL deleted the yoshrk-dss-validation_pp branch December 6, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RAVENv2.2 for RAVENv2.2 Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK] Adding DSS Capabilities to Validation Code
4 participants