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

Matter_Engine: Improve Density Management for Physical materials #2935

Merged

Conversation

IsakNaslundBh
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh commented Oct 28, 2022

NOTE: Depends on

BHoM/BHoM#1443

Issues addressed by this PR

  • Align with Density added to Material
  • Add methods for extracting Density from a set of IDensityProviders
  • Add method to set density to a material based on its Properties
  • Update Material Create methods to scan Properties used to create for Density

Test files

https://burohappold.sharepoint.com/:f:/s/BHoM/Eg7Y5NfGykJKr5Bc1wfki6ABOXhTCqmW-pr3oNrYvFzKYA?e=tJxtni

Changelog

Additional comments

@IsakNaslundBh IsakNaslundBh self-assigned this Oct 31, 2022
@IsakNaslundBh IsakNaslundBh added the type:feature New capability or enhancement label Oct 31, 2022
@IsakNaslundBh IsakNaslundBh changed the title Matter_Engine: Improve Density Management (extraction and setting) for Physical materials Matter_Engine: Improve Density Management for Physical materials Oct 31, 2022
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check null-handling
@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 1, 2022

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

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance
  • check null-handling
  • check unit-tests

@IsakNaslundBh IsakNaslundBh marked this pull request as ready for review November 1, 2022 10:23
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 1, 2022

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

  • check unit-tests

@IsakNaslundBh
Copy link
Contributor Author

Have investigated the failing UTs for VolumetricMaterialTakeoff and MaterialComposition and both boils down to the same reason for the failure. Both failing cases are that the density is set to 7850 when run on this branch, while on main the non-existing value returned is 0:

image

Creating a composition or takeoff straight from a structural material it is valid to pass the density to the physical material, hence the change in behaviour is expected and an improvement.

Will for those reasons regenerate those UTs to what is returned on this branch.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 1, 2022

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

  • check unit-tests

There are 2 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 1, 2022

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

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 2 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 1, 2022

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

  • check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

@IsakNaslundBh just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @IsakNaslundBh on BHoM

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

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

  • check unit-tests

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

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

  • check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

@IsakNaslundBh just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @IsakNaslundBh on BHoM

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

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

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance

There are 9 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check null-handling
@BHoMBot check serialisation
@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

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

  • check null-handling
  • check serialisation
  • check installer

There are 10 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check core

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 2, 2022

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

  • check core

There are 17 requests in the queue ahead of you.

Copy link
Member

@al-fisher al-fisher left a comment

Choose a reason for hiding this comment

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

Approving based on review and commentary in oM PR - see BHoM/BHoM#1443

@al-fisher
Copy link
Member

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Nov 3, 2022

@al-fisher to confirm, the following actions are now queued:

  • check ready-to-merge

There are 2 requests in the queue ahead of you.

@al-fisher al-fisher merged commit 005bb88 into main Nov 3, 2022
@al-fisher al-fisher deleted the Physical_oM-#1442-DensityManagementForPhysicalMaterials branch November 3, 2022 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants