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

ETABS_Toolkit: Set material design parameters during push #405

Conversation

JosefTaylor
Copy link
Contributor

@JosefTaylor JosefTaylor commented Sep 6, 2022

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 6, 2022

@JosefTaylor 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

@mchaf
Copy link

mchaf commented Sep 6, 2022

I looked at this today: Overall, the compressive force, modulus of elasticity and other design parameters came in without an issue. Still, there were some other things that might be worth fixing.

First, for both steel and concrete, there are parameters that are not defined by BHoM Materials (effective yield stress, expected rupture, nonlinear material data, etc) that need to be set. Firstly, do we want want to set a value, or do we want to make it 0? If we do set a value, we should be very clear about which parameters are being set by us, and not the user.

Secondly, within the Material Property Design Data for steel and concrete, the "grade" parameter does not take the name of the material. It should, to avoid confusion. Upon talking to Joe, it seems like this is a problem on the ETABS side to figure out and dive in to.

I'll be happy to look at it again after we solve this, but the overall functionality of transferring compressive strength and modulus of elasticity (the two more important properties) works, which is a win :)

@IsakNaslundBh IsakNaslundBh added the type:feature New capability or enhancement label Sep 8, 2022
Changed _Read to check for types which share an interface (i.e. Steel, Concrete are both IMaterialFragment)
Changed ReadMaterial to conform to SAP2000 which works a bit nicer
Changed CreateMaterial to try ETABS.AddMaterial() first, which is the new method which theoretically looks in the database, however it would be difficult to implement. If AddMaterial fails, it goes to SetMaterial, which is the old method.
Change the default push type to CreateNonExisting, so that if materials are pre-loaded, they are not altered (they would be altered in undesirable ways).
@JosefTaylor
Copy link
Contributor Author

Diving into the issues Mel raised uncovered a few more - if a user pushed a material (not a bar or property containing a material), it would trigger Create, instead of Update, and the Create checked if it already existed, which is normally superfluous. Fixing this meant that properties on existing materials are overwritten, which is usually not desired. So, I changed the default push type to CreateNonExisting, so that a user could pre-load any materials they need to use before pushing dependent types, and those materials will not be changed by an unintended update.
Unfortunately, it is apparently not possible to change the 'grade' property of a material, so when adding a material, this will be shown as the default grade for the type, i.e. 'Grade 50' for steel and "f'c 4000 psi" for concrete.
Also, for steel, ETABS defines two parameters EFy and EFu, which BHoM does not define. Rather than leave these as the value set by ETABS (always from A992 Grade 50), they are set equal to Fy and Fu respectively, and the user can change them manually in ETABS if needed.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 8, 2022

@JosefTaylor 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.

@JosefTaylor
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 8, 2022

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

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

There are 22 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 8, 2022

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 8, 2022

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

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

General functionality works well, but some of the changes are a bit to american code centric, and makes pushing items raise warnings that does not make sense for for example Eurocode.

Think this needs a quick discussion before being merged in.

Etabs_Adapter/CRUD/Create/Material.cs Show resolved Hide resolved
Etabs_Adapter/CRUD/Create/Material.cs Outdated Show resolved Hide resolved
@JosefTaylor
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 9, 2022

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

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

There are 37 requests in the queue ahead of you.

Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

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

Happy now with the changes made!

@IsakNaslundBh
Copy link
Contributor

@BHoMBot check copyright-compliance
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 9, 2022

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

  • check copyright-compliance
  • check ready-to-merge

There are 29 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor

@BHoMBot check copyright-compliance
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 12, 2022

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

  • check copyright-compliance
  • check ready-to-merge

There are 16 requests in the queue ahead of you.

@FraserGreenroyd
Copy link
Contributor

Following a request by @IsakNaslundBh , I have reviewed this PR and its mergeability given the current feature freeze for the 5.3 beta release, and am happy this PR conformed to the guidelines laid out for the previous sprint and may be merged today prior to the test beta artefacts being produced for inclusion in the 5.3 beta.

@IsakNaslundBh IsakNaslundBh merged commit 01be760 into main Sep 12, 2022
@IsakNaslundBh IsakNaslundBh deleted the ETABS_Toolkit-#378-set-design-parameters-during-material-push branch September 12, 2022 08:16
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.

Material Strength not set in Push
4 participants