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

fix snap list for blueprints assemblies #783

Merged
merged 4 commits into from
Jul 20, 2022
Merged

fix snap list for blueprints assemblies #783

merged 4 commits into from
Jul 20, 2022

Conversation

onufer
Copy link
Member

@onufer onufer commented Jul 19, 2022

Description

Fixing snap list for blueprints assemblies


Checklist

  • The code is understandable and maintainable to people beyond the author.
  • Tests have been added/updated to verify that the new or changed code works.
  • There is no commented out code in this PR.
  • The commit message follows good practices.
  • All docstrings are still up-to-date with these changes.

@onufer onufer requested a review from jakehader July 19, 2022 23:20
@jakehader
Copy link
Member

@john-science we are still debugging some issues when loading from a database and the treatment of updateAxialMesh. @onufer will update this once we have resolved those issues and the PR will be ready.

@john-science john-science added the enhancement New feature or request label Jul 20, 2022
@jakehader
Copy link
Member

@john-science - The changes here are good and help us get DB loading operational. There are a lot of complexities with the mesh treatment at different loading conditions (from settings/blueprints or from a database) that we need to resolve throughout the code base. This is a good first step and patch for the on-going work here, but @albeanth and I will work together on prioritizing the issues more globally and then proposing an "ARMI mesh refactor/clean-up" project that will help us to get better integration testing for the framework and unit testing on the various ways we can initialize a system model.

I will leave this up to you or @onufer to Squash and Merge, as I don't want to jump ahead without your review and making sure that it doesn't have some impacts on the current in-progress PRs.

@jakehader jakehader added the bug Something is wrong: Highest Priority label Jul 20, 2022
@john-science
Copy link
Member

@jakehader Also, Tony and I were talking yesterday and it sounds like there has not been much/any testing of the new detailed axial expansion features when it comes to reading from a Database. So I think that needs to be tested at some point, wherever that fits on the schedule.

@keckler
Copy link
Member

keckler commented Jul 20, 2022

Tony and I were talking yesterday and it sounds like there has not been much/any testing of the new detailed axial expansion features when it comes to reading from a Database

I have been loading axially-expanded cores from the DB. I haven't looked in super detail at it, but it appears to be working in the sense that the assemblies are expanded.

@jakehader
Copy link
Member

@jakehader Also, Tony and I were talking yesterday and it sounds like there has not been much/any testing of the new detailed axial expansion features when it comes to reading from a Database. So I think that needs to be tested at some point, wherever that fits on the schedule.

This is not with detailedAxialExpansion.

@jakehader jakehader merged commit 3d1ef4e into main Jul 20, 2022
@jakehader jakehader deleted the fixBlueprintsSnap branch July 20, 2022 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants