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

Alfoa/codeinterfaces #1366

Merged
merged 40 commits into from
Dec 8, 2020
Merged

Alfoa/codeinterfaces #1366

merged 40 commits into from
Dec 8, 2020

Conversation

alfoa
Copy link
Collaborator

@alfoa alfoa commented Nov 3, 2020


Pull Request Description

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

Closes #1360
Closes #1274
Closes #1079 (the duplicated values must be handled at the code interface level)
Closes #446
Closes #1367

What are the significant changes in functionality due to this change request?

Addition of the possibility to return the output data directly from the finalizeCodeOutput. In this way,
we avoid the double IO of
reading Code Output -> memory -> creation of CSV -> RAVEN reading the CSV -> output in memory


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.

alfoa and others added 16 commits November 3, 2020 13:55
* Adding ability to sign the .so files. (#1362)

Adding ability to sign the .so files.. This will be needed in future mac os releases.

* debug

* fix dymolainterface

* revert code interface changes

* update rounding error for time index

* fix index for duplicated values

Co-authored-by: Joshua J. Cogliati <joshua-cogliati-inl@users.noreply.github.com>
…ting (in the interface) that was skipping the last time step (10000)
Copy link
Contributor

@joshua-cogliati-inl joshua-cogliati-inl left a comment

Choose a reason for hiding this comment

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

There are several things that need changing. Someone will have to re-review after they are made.

The basic changes look good, but there are several accidental changes that need to be reverted.

framework/CodeInterfaceBaseClass.py Show resolved Hide resolved
framework/CodeInterfaces/RELAP5/RELAPparser.py Outdated Show resolved Hide resolved
framework/Files.py Show resolved Hide resolved
scripts/TestHarness/testers/RavenFramework.py Outdated Show resolved Hide resolved
scripts/TestHarness/testers/RavenPython.py Outdated Show resolved Hide resolved
tests/framework/unit_tests/utils/testRandomUtils.py Outdated Show resolved Hide resolved
@moosebuild
Copy link

Job Test mac on 1a3f334 : invalidated by @alfoa

@alfoa
Copy link
Collaborator Author

alfoa commented Dec 4, 2020

@joshua-cogliati-inl I addressed your comments.

@joshua-cogliati-inl
Copy link
Contributor

@joshua-cogliati-inl I addressed your comments.

Thanks, I'll take a look today.

Copy link
Contributor

@joshua-cogliati-inl joshua-cogliati-inl left a comment

Choose a reason for hiding this comment

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

This is much improved. I approve.

framework/CodeInterfaces/RELAP5/RELAPparser.py Outdated Show resolved Hide resolved

# in general, we will use Crow for now, but let's make it easy to switch just in case it is helpful eventually.
# in general, we will use Crow for now, but let's make it easy to switch just in case it is helpfull eventually.
Copy link
Contributor

Choose a reason for hiding this comment

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

For what it is worth, helpful is the more standard spelling.

@@ -41,7 +41,7 @@
2500000 inlet tmdpvol
2500101 1.0 1.0 0.0 0.0 0.0 0.0 0.0 0.0 0000000
2500200 003
2500201 0.0 6.39e+06 530.245239193
2500201 0.0 6.39e+06 5.30245e+02
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a comment or a piece of data that is used by Relap5?

@moosebuild
Copy link

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

restarted civet

@joshua-cogliati-inl
Copy link
Contributor

When the tests pass, this is ready to go.

@moosebuild
Copy link

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

deleted raven directory

@moosebuild
Copy link

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

Failed tests\framework\CodeInterfaceTests\genericInterfaceParallel

@joshua-cogliati-inl joshua-cogliati-inl merged commit cd67dba into devel Dec 8, 2020
@alfoa alfoa deleted the alfoa/codeinterfaces branch December 8, 2020 18:23
@alfoa alfoa added the RAVENv2.1 All tasks and defects that will go in RAVEN v2.1 label Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RAVENv2.1 All tasks and defects that will go in RAVEN v2.1
Projects
None yet
4 participants