Skip to content

Fixing inconsistent sign in the RM3 example#58

Closed
jleonqu wants to merge 3 commits intoWEC-Sim:devfrom
jleonqu:fixSignRM3Example
Closed

Fixing inconsistent sign in the RM3 example#58
jleonqu wants to merge 3 commits intoWEC-Sim:devfrom
jleonqu:fixSignRM3Example

Conversation

@jleonqu
Copy link
Copy Markdown
Contributor

@jleonqu jleonqu commented Feb 8, 2024

This is an update to the RM3 example based on this PR in the WEC-Sim repo: PR #1221
Which was based on this Issue from the WEC-Sim repo: Issue #1218

@dforbush2 dforbush2 self-requested a review February 9, 2024 22:43
@dforbush2
Copy link
Copy Markdown
Contributor

This fix works. I do request that the run time is set back to ~400 seconds so that the test cases do not need to run to 2000 s, but the long run is was a good test case (I do not have write access to this fork).

Ready to merge.

@H0R5E H0R5E force-pushed the dev branch 3 times, most recently from 7760345 to 2125949 Compare February 14, 2024 10:41
@akeeste akeeste self-requested a review February 28, 2024 15:55
@akeeste
Copy link
Copy Markdown
Contributor

akeeste commented Feb 28, 2024

@jleonqu Based on the error message in the runner, it seems there's still an issue with the fix in the PTO-Sim library.

Warning message:
[Warning: Result of power is undefined in 'RM3_cHydraulic_PTO/PTO-Sim/Gas-Charged Hydraulic Accumulator/Math Function1'.]

The case ends up failing because the simulation goes unstable.

@jleonqu
Copy link
Copy Markdown
Contributor Author

jleonqu commented Mar 5, 2024

@akeeste PR #1236 from the WEC-Sim repo should fix this issue with the test that is failing.

@dforbush2
Copy link
Copy Markdown
Contributor

dforbush2 commented Mar 19, 2024

I successfully ran this example in windows 2023b... not sure why the test is not passing but can confirm that this works (using the library change in WEC-Sim PR in #1236

It seems that when #1236 is merged (and pushed to Dev) these tests should pass. Running this sim on dev branch will cause the displayed error.

@jleonqu
Copy link
Copy Markdown
Contributor Author

jleonqu commented Mar 20, 2024

Closing this PR. The PR #60 was created to rebase this PR to the main branch.

@jleonqu jleonqu closed this Mar 20, 2024
kmruehl pushed a commit that referenced this pull request Apr 11, 2024
)

* Fixing the inconsistent sign in the RM3 example

* Fix sign bug on OSWEC Hydraulic PTO examples
kmruehl added a commit that referenced this pull request Sep 16, 2024
* Rebase PR #58 to main - Fixing inconsistent sign in the RM3 example (#60)

* Fixing the inconsistent sign in the RM3 example

* Fix sign bug on OSWEC Hydraulic PTO examples

* Update wecSimInputFile.m

* Add files via upload

Fixes a bug, the main .xls file is replaced with the main .xls file for this example

* Fixes bugs with PTO-Sim

* fix in wecSimInputFile for morisonElement

* Control Test Folder Selection with Dynamic Workflows (#63)

* Test dispatch workflow with automatic folder discovery

* Set MATLAB version to 2023b

* I forgot the R

* Check out the correct branch for test discovery

* Try it without toJSON

* Update workflow for other events

Also remove dev workflow as it should work on any branch now.

* Need fetch depth for git diff

* Set branch correctly and don't run of no folder

* Be more explicit with the empty condition

* Try and examine what is in the folder variable

* Try checking againt empty JSON array string

* Test diffed folder

* Try diffing two folders and echoing reusable workflow inputs

* Try reordering the matrix

* Try using a fixed os

* Try to prevent included cases running when no folder

* Curb my ambition!

* Exclude matching cases rather than include

* Remove the include conditions

* Record required MATLAB packages in each test directory

* Export products into test matrix

* Dynamically create include cases

* Debug input names

* Examine the include input variables

* Fix length one include array

* Consider the base ref for pull requests

* Explicitly set the checkout repo

* Include repository in the reusable test inputs

* Use full repo name

* Set runs-on from matrix

* Update README

* Update badges

* Link to process-events workflow

* Try adding  concurrency term to stop runs for each PR commit

* Switch to shields.io badges to customize the labels

Also prettify the file.

* Split process events workflow to allow sensible badges

Badges set to a particular branch don't capture the status of
repository dispatch events.

* Refactor test workflows

Test workflows are refactored to split out those triggered by WEC-Sim
commits (wec-sim-***-tests) from those triggered by modifications to the
applications (changed-tests).

---------

Co-authored-by: Jorge Leon <72461917+jleonqu@users.noreply.github.com>
Co-authored-by: Mohamed Shabara <84589678+MShabara@users.noreply.github.com>
Co-authored-by: Shabara <mshabara@nrel.gov>
Co-authored-by: Forbush <dforbus@sandia.gov>
Co-authored-by: Mathew Topper <damm_horse@yahoo.co.uk>
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.

3 participants