Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355

IsakLarbornBH · 2021-02-26T12:52:03Z

NOTE: Depends on

Graphics_oM: Add GradientOptions and GradientCenteringOptions BHoM#1232

Issues addressed by this PR

Closes #2318
Closes #2319

Result_Engine:
Adds DisplayMeshResults, which displays IMeshResults on IMeshes, and MapResults, which maps IResults to IBHoMObjects using common identifiers such as object IDs.
Graphics_Engine:
Adds methods for dealing with GradientOptions objects from BHoM/BHoM#1232.

All methods added have been adapted from methods that were previously in the StructuralEngineering_Toolkit.

Test files

A messy but functional test file can be found here. This will allow you to push a simple test model to Robot, execute an analysis and then pull the results. You can of course use it with your own Robot file and just pull the results from that, if you prefer.
MeshResult Test.zip

If you don't want to use Robot, here is a simpler test file with some internalised FEMeshes and results:
MeshResult Internalised.zip

Note that both of these contain the DisplayMeshResultsWorkaround method which has been removed since #2367 is now merged, fixing the issue that was being worked around.

Unit tests have been added for DisplayMeshResults, which calls MapResults and all the GradientOptions methods.

Additional comments

This pull request is one in a series that should, to minimise disruption, be merged on the same day.

Graphics_oM: Add GradientOptions and GradientCenteringOptions BHoM#1232
Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355
BuroHappoldEngineering/StructuralEngineering_Toolkit#419

IsakLarbornBH · 2021-03-09T10:28:17Z

Removing draft status now on these PRs as they have now been fully implemented, and with #2367 there is now, from my point of view, no reason not to merge these pull requests.

Unit tests have been added for DisplayMeshResults(), which also tests MapResults and all the gradient methods in #2354.

This will fail all checks until #2354 is merged - not sure what the proper way is to create PRs spanning multiple projects in the same repository.

FraserGreenroyd · 2021-04-01T23:13:16Z

@IsakLarbornBH I feel this PR has become a bit mammoth looking at the commit history and the comments in the original issue regarding related PRs. It might be worth us having a catch up next sprint to go through this and get it ready so we can make sure this gets adequate reviewing resource from @IsakNaslundBh and others? 😄

IsakLarbornBH · 2021-04-08T08:28:59Z

@IsakLarbornBH I feel this PR has become a bit mammoth looking at the commit history and the comments in the original issue regarding related PRs. It might be worth us having a catch up next sprint to go through this and get it ready so we can make sure this gets adequate reviewing resource from @IsakNaslundBh and others? 😄

@FraserGreenroyd For sure, though it was always going to be a pretty big one. Although most of the commits are tiny edits, or adding and then removing temporary workarounds I had to use before I had migrated everything, so the commit history is a bit inflated and not as bad as it may seem at first glance.

Still though, definitely worth it to have a catch-up! This PR is going to take some work to review, and we should start early on that so we can be sure to get it merged in 4.2.

rolyhudson · 2021-04-08T08:59:44Z

Hand up here from me.
I am keen to help get this series of PRs merged as part of a bigger (epic) foundational workflow related to representation and visualisation.
Think we need to avoid this though:

You must manually create a branch merging this branch and the branch referenced in #2354 in order to test this.

I say it is better to combine this branch with #2354 and close this PR.

Also happy to be part of any discussion if deemed necessary to move this forward

IsakLarbornBH · 2021-04-08T09:14:05Z

I say it is better to combine this branch with #2354 and close this PR.

I'd be more than happy to do so, though I would keep this PR and close #2354 instead, as this is the main PR.

Edit: Merged it.

rolyhudson · 2021-04-08T09:41:53Z

Graphics_Engine/Modify/ApplyGradientOptions.cs

+        [PreviousVersion("4.2", "BH.Engine.Graphics.Query.AutoRange(BH.oM.Graphics.GradientOptions, System.Collections.Generic.IEnumerable<System.Double>)")]
+        [PreviousVersion("4.2", "BH.Engine.Graphics.Query.CenteringOptions(BH.oM.Graphics.GradientOptions)")]
+        [PreviousVersion("4.2", "BH.Engine.Graphics.Query.DefaultGradient(BH.oM.Graphics.GradientOptions, System.String)")]
+        public static GradientOptions ApplyGradientOptions(this GradientOptions gradientOptions, IEnumerable<double> allValues = null, string defaultGradient = "BlueToRed")


Are there really 3 previous versions?

Originally they were 3 separate methods doing three steps of the process, but they were made in a very particular way and essentially had to be used together in a certain order, so I merged them into one. I doubt anybody was calling them from the UI anyway, so versioning might not be necessary, but if they were it does actually make sense to version them all to this merged one since it's made in such a way as to not override manually set parameters.

OK. Yes I have seen them in the StructuralEnginnering_Toolkit. Lets see of the versioning check likes that, I've not tried it before. If not the MessageForDeleted in the versioning json file may be an option if they are not that widely used.

Results_Engine/Results_Engine.csproj

rolyhudson · 2021-04-08T10:08:38Z

Results_Engine/Query/DisplayMeshResults.cs

+        }*/
+
+        /***************************************************/
+


Any good reason to keep this commented out private method? Or should it be cleaned out?

This is @IsakNaslundBh who attempted to create a more optimised replacement for my calls to PropertyValue, but we never got it to work and when I read up about it later I got the impression that it wouldn't work in this case, though it was a while ago and I don't remember the details. Personally I haven't noticed the PropertyValue calls slowing this method down one bit, so I'd be happy to remove it.

I think you can delete this. If the performance becomes a problem, a separate PR will target this.

IsakLarbornBH · 2021-04-08T10:38:08Z

@BHoMBot check core

bhombot-ci · 2021-04-08T10:38:09Z

@IsakLarbornBH to confirm, check core task is now queued.

IsakLarbornBH · 2021-04-08T15:18:25Z

@BHoMBot check compliance

IsakNaslundBh · 2021-06-01T09:25:50Z

Unfortunately had to locally rebase and force push this as well due to github complaining about conflicts. Rebase locally went with 0 hickups and no conflicts showing up. Not sure why Github did not like it, but hopefully will be fixed now.

IsakNaslundBh · 2021-06-01T09:26:38Z

@BHoMBot check required

bhombot-ci · 2021-06-01T09:26:39Z

@IsakNaslundBh to confirm, the following checks are now queued:

code-compliance
documentation-compliance
project-compliance
core
null-handling
serialisation
installer
versioning

IsakNaslundBh · 2021-06-01T09:27:46Z

@BHoMBot check copyright-compliance

bhombot-ci · 2021-06-01T09:27:47Z

@IsakNaslundBh to confirm, the following checks are now queued:

copyright-compliance

There are 12 requests in the queue ahead of you.

IsakNaslundBh · 2021-06-01T09:28:01Z

@BHoMBot check dataset-compliance

bhombot-ci · 2021-06-01T09:28:02Z

@IsakNaslundBh to confirm, the following checks are now queued:

dataset-compliance

There are 14 requests in the queue ahead of you.

IsakNaslundBh

Re-approving after testing again.

Local rebase did not throw up any conflicts, so think previous approvals are still safe as well!

bhombot-ci · 2021-06-01T10:01:03Z

FAO: @FraserGreenroyd @IsakNaslundBh is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is null-handling.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a CI/CD instruction. I am authorising dispensation to be granted on check ref. 2717219160

IsakNaslundBh · 2021-06-01T10:01:19Z

@BHoMBot this is a CI/CD instruction. I am authorising dispensation to be granted on check ref. 2717219160

bhombot-ci · 2021-06-01T10:01:21Z

@IsakNaslundBh I have now provided a passing check on reference 2717219160 as requested.

IsakNaslundBh · 2021-06-01T10:01:35Z

@BHoMBot check ready-to-merge

bhombot-ci · 2021-06-01T10:01:36Z

@IsakNaslundBh to confirm, the following checks are now queued:

ready-to-merge

IsakLarbornBH added the status:do-not-merge For instance, test PR, for discussion, or dependant PRs not ready for merge label Feb 26, 2021

IsakLarbornBH self-assigned this Feb 26, 2021

This was referenced Feb 26, 2021

Graphics_Engine: Add gradient options methods #2354

Closed

Graphics_oM: Add GradientOptions and GradientCenteringOptions BHoM/BHoM#1208

Closed

BHoM deleted a comment from bhombot-ci bot Mar 2, 2021

IsakLarbornBH removed the status:do-not-merge For instance, test PR, for discussion, or dependant PRs not ready for merge label Mar 9, 2021

IsakLarbornBH marked this pull request as ready for review March 9, 2021 10:23

IsakLarbornBH requested review from adecler, al-fisher, FraserGreenroyd, IsakNaslundBh and rwemay as code owners March 9, 2021 10:23

IsakLarbornBH changed the title ~~Result_Engine: Add DisplayMeshResults and MapResults~~ Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions Apr 8, 2021

rolyhudson reviewed Apr 8, 2021

View reviewed changes

Results_Engine/Results_Engine.csproj Outdated Show resolved Hide resolved

rolyhudson reviewed Apr 8, 2021

View reviewed changes

IsakLarbornBH mentioned this pull request Apr 8, 2021

Graphics_oM: Add GradientOptions and GradientCenteringOptions BHoM/BHoM#1232

Merged

IsakLarbornBH and others added 10 commits June 1, 2021 11:24

Dummy commit to re-trigger checks

cc2a65c

Align with renamed GradientOptions bounds

d02adb2

Rename method inputs to align with property name change

3e0142d

Dummy commit to re-trigger checks

50e03be

Remove unused references and using statements

091525f

Remove commented out optimisation attempts

4e2c674

Update CSProject compliance

060dc0d

Add null handling to GradientOptions methods

4a88e59

Add null handling to DisplayMeshResults and MapResults

0a58ef3

Add missing copyright statement

e33ffd1

IsakNaslundBh dismissed their stale review via e33ffd1 June 1, 2021 09:24

IsakNaslundBh force-pushed the Results_Engine-#2318-MigrateMeshResultMethods branch from 40314f0 to e33ffd1 Compare June 1, 2021 09:24

IsakNaslundBh approved these changes Jun 1, 2021

View reviewed changes

IsakNaslundBh merged commit 954796e into master Jun 1, 2021

IsakNaslundBh deleted the Results_Engine-#2318-MigrateMeshResultMethods branch June 1, 2021 10:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355

Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355

IsakLarbornBH commented Feb 26, 2021 •

edited by FraserGreenroyd

Loading

IsakLarbornBH commented Mar 9, 2021 •

edited

Loading

FraserGreenroyd commented Apr 1, 2021

IsakLarbornBH commented Apr 8, 2021

rolyhudson commented Apr 8, 2021

IsakLarbornBH commented Apr 8, 2021 •

edited

Loading

rolyhudson Apr 8, 2021

IsakLarbornBH Apr 8, 2021 •

edited

Loading

rolyhudson Apr 8, 2021 •

edited

Loading

rolyhudson Apr 8, 2021

IsakLarbornBH Apr 8, 2021

alelom May 5, 2021

IsakLarbornBH commented Apr 8, 2021

bhombot-ci bot commented Apr 8, 2021

IsakLarbornBH commented Apr 8, 2021

IsakNaslundBh commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh left a comment

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355

Result_Engine, Graphics_Engine: Add DisplayMeshResults, MapResults, and methods for GradientOptions #2355

Conversation

IsakLarbornBH commented Feb 26, 2021 • edited by FraserGreenroyd Loading

NOTE: Depends on

Issues addressed by this PR

Test files

Additional comments

IsakLarbornBH commented Mar 9, 2021 • edited Loading

FraserGreenroyd commented Apr 1, 2021

IsakLarbornBH commented Apr 8, 2021

rolyhudson commented Apr 8, 2021

IsakLarbornBH commented Apr 8, 2021 • edited Loading

rolyhudson Apr 8, 2021

Choose a reason for hiding this comment

IsakLarbornBH Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

rolyhudson Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

rolyhudson Apr 8, 2021

Choose a reason for hiding this comment

IsakLarbornBH Apr 8, 2021

Choose a reason for hiding this comment

alelom May 5, 2021

Choose a reason for hiding this comment

IsakLarbornBH commented Apr 8, 2021

bhombot-ci bot commented Apr 8, 2021

IsakLarbornBH commented Apr 8, 2021

IsakNaslundBh commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh left a comment

Choose a reason for hiding this comment

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakNaslundBh commented Jun 1, 2021

bhombot-ci bot commented Jun 1, 2021

IsakLarbornBH commented Feb 26, 2021 •

edited by FraserGreenroyd

Loading

IsakLarbornBH commented Mar 9, 2021 •

edited

Loading

IsakLarbornBH commented Apr 8, 2021 •

edited

Loading

IsakLarbornBH Apr 8, 2021 •

edited

Loading

rolyhudson Apr 8, 2021 •

edited

Loading