Set Performance Test Suite graphs to Manual Runtype #9604
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d debug menu. Deleted debug menu items.
|
We should just open graphs with force manual mode on if we can
… On Mar 27, 2019, at 6:27 PM, Scott Mitchell ***@***.***> wrote:
Purpose
@saintentropy noticed that some of the test suite graphs were set to Automatic, which caused both graph to both Open and Run in the OpenModelBenchmark rather than wait for the RunModelBenchmark to Run.
Declarations
Check these if you believe they are true
The code base is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning, and are documented in the API Changes document.
FYIs
@saintentropy @mjkkirschner @reddyashish @QilongTang @alfarok @ColinDayOrg
You can view, comment on, or merge this pull request online at:
#9604
Commit Summary
update
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Brought truncated search results and full query search out from behind debug menu. Deleted debug menu items.
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Pull upstream master
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Merge branch 'master' of https://github.com/DynamoDS/Dynamo
Added test graphs. Added readme with test graph info
Merge branch 'master' of https://github.com/DynamoDS/Dynamo into DYN-1545
Moved Heterogenous Input test graphs
Update readme
xml -> json
Added TspinesToMesh graph
Added FFITarget.dll. Added Element Binding graph.
Removed SinuousTower and LotsOfStuff
Changed graph Dynamo versions to earliest possible. Renamed graphs
rename ElementBinding image
Update readme
Set performance test suite graphs to manual mode
Merge upstream master
File Changes
M tools/Performance/DynamoPerformanceTests/graphs/CodeBlocks.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/HeterogeneousInputsFirst.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/HeterogeneousInputsLast.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/HomogeneousInputs.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/LotsOfColoredStuff.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/Math.dyn (2)
M tools/Performance/DynamoPerformanceTests/graphs/Points_40x40x40.dyn (2)
Patch Links:
https://github.com/DynamoDS/Dynamo/pull/9604.patch
https://github.com/DynamoDS/Dynamo/pull/9604.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
This LG, @mjkkirschner I can also add new APIs to the DynamoModelTestBase to open graph in force manual mode, currently such API does not exist for performance test framework to use. |
scottmitchell
commented
Mar 28, 2019
| @@ -130,10 +130,10 @@ protected void EmptyScheduler() | |||
| } | |||
| } | |||
|
|
|||
| protected void OpenModel(string relativeFilePath) | |||
| protected void OpenModel(string relativeFilePath, bool forceManualExecutionMode = false) | |||
There was a problem hiding this comment.
@mjkkirschner @QilongTang Can we do this? Or do we need to add a new method?
There was a problem hiding this comment.
@QilongTang I can remove this if you would rather just handle it as part of your task.
There was a problem hiding this comment.
I think this is the way to do it.
There was a problem hiding this comment.
I guess we need to consider that this might effect others who have written tests against the test base classes.....
I guess it's safest to add a new method unless you want to test this or do some research on the API compatibility of adding default arguments to protected methods?
There was a problem hiding this comment.
That's good, can you run self CI just to make sure?
There was a problem hiding this comment.
For the record, this did pass the self CI, but I'm going to remove this change for now and just merge the RunType changes.
This reverts commit a05d975.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
@saintentropy noticed that some of the test suite graphs were set to Automatic, which caused both graph to both Open and Run in the
OpenModelBenchmarkrather than wait for theRunModelBenchmarkto Run.Declarations
Check these if you believe they are true
*.resxfilesFYIs
@saintentropy @mjkkirschner @reddyashish @QilongTang @alfarok @ColinDayOrg