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

Pull NodeResult returns multiple mode results for seismic cases #261

Closed
njsmithies opened this issue Sep 23, 2019 · 6 comments · Fixed by #262
Closed

Pull NodeResult returns multiple mode results for seismic cases #261

njsmithies opened this issue Sep 23, 2019 · 6 comments · Fixed by #262
Assignees
Labels
type:bug Error or unexpected behaviour type:feature New capability or enhancement

Comments

@njsmithies
Copy link

njsmithies commented Sep 23, 2019

When pulling NodeResults for a Robot seismic case multiple modal results are returned in no particular order. Combined CQC or SRSS result is not returned.

Pulling seismic results case should return combined (CQC or SRSS...) result only.

To return modal results a mode property is required in NodeResult.

Test File: https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/01_Test%20Scripts/Robot_Toolkit/%20Robot_Toolkit-Issue%20261-Pull%20NodeResult%20Seismic%20Case?csf=1&e=Bx9RHs

@njsmithies njsmithies added type:bug Error or unexpected behaviour type:feature New capability or enhancement labels Sep 23, 2019
@rwemay
Copy link
Member

rwemay commented Sep 23, 2019

Thanks @njsmithies , can you also please save a copy of the model in the test script library? There is a subfolder for issues, and you can create a new folder in there with the issue number above.

@rwemay
Copy link
Member

rwemay commented Sep 23, 2019

@IsakNaslundBh , the result class doesn't have a 'Mode' property - I think we should add, as this isn't only relevant for modal analysis results, but seismic (response spectrum analysis) too. In the interim I'm planning to make the Robot_Toolkit report results from the combined seismic results. The 'Modal' property, if there was one, in this case would return 'CQC' for all results (which is the name of the modal composition method).

@IsakNaslundBh
Copy link
Contributor

@rwemay I agree. We could make use of and rename the TimeStep property potentially to handle this. Calling it just Step or similar could maybe be generic enough? Could maybe do this (add to timeStep) as a fix for now?

If not clear enough (not sure it is really..), might have to resort to add in a Mode property for all of our structural results, agreed.

@rwemay
Copy link
Member

rwemay commented Sep 23, 2019

Hey @IsakNaslundBh , as Skyped, would be great to reuse an existing property, but I think in this case we'd need a specific Mode string property - as we'd need to store/know whether it's a time history or a modal analysis in any case, apart from mode being stored as timestep being a bit confusing.

The only other way I thought was to create a new result class(es) specific to the analysis type, but not sure we want to go down that route......

@rwemay
Copy link
Member

rwemay commented Sep 24, 2019

Hey @njsmithies , I've pushed an update to the branch below. I'm having internet issues as on very weak connection so I might struggle to test this today (can't get a license/open models). If you've a few mins, feel free to give it a go, but if it fails first time, wait for me to test later. @johannaisak , @JonathanNillius FYI

@JonathanNillius
Copy link
Contributor

The branch works! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants