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

enhancements to matset to_silo #677

Merged
merged 3 commits into from Jan 13, 2021
Merged

Conversation

cyrush
Copy link
Member

@cyrush cyrush commented Jan 12, 2021

No description provided.

@cyrush cyrush requested a review from xjrc January 12, 2021 23:31
@cyrush
Copy link
Member Author

cyrush commented Jan 12, 2021

@xjrc let me know what you think, FYI -- I want to improve some of the tests

@coveralls
Copy link

coveralls commented Jan 13, 2021

Coverage Status

Coverage increased (+0.2%) to 86.5% when pulling 2559e1d on task/2021_01_matset_to_silo_updates into 51d0b71 on develop.

@cyrush
Copy link
Member Author

cyrush commented Jan 13, 2021

One issue I need to resolve, the size of the matlist ends up being too big in the uni-buffer case:

topology: "topo"
material_map: 
  circle_a: 1
  circle_b: 2
  circle_c: 3
  background: 0
volume_fractions: [1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.333333333333333, 0.333333333333333, 0.333333333333333, 1.0, 1.0, 1.0, 0.5, 0.5, 1.0]
material_ids: [0, 0, 0, 0, 0, 0, 3, 0, 0, 1, 1, 2, 3, 2, 0, 1, 1, 2, 2]
sizes: [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 3, 1, 1, 1, 2, 1]
offsets: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 13, 14, 15, 16, 18]


topology: "topo"
material_map: 
  circle_a: 1
  circle_b: 2
  circle_c: 3
  background: 0
matlist: [1, 1, 1, 1, 1, 1, 4, 1, 1, 2, -1, 3, 1, 2, -4, 3, 0, 0, 0]
mix_next: [2, 3, 0, 5, 0]
mix_mat: [1, 2, 3, 1, 2]
mix_vf: [0.333333333333333, 0.333333333333333, 0.333333333333333, 0.5, 0.5]

(see the extra three 0s)

I think this value is coming from:


        blueprint::o2mrelation::O2MIterator mat_iter(mat_vfs);
        matset_num_elems = mat_iter.elements(O2MIndex::ONE);

I could just use the number of elements of the offsets or sizes, but I want to make sure the O2M stuff is working as we expect.

Copy link
Member

@xjrc xjrc left a comment

Choose a reason for hiding this comment

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

Looks good Cyrus, but I think there's still one thing incorrect with my initial implementation; please take a look at my comment and let me know what you think. Once we get that issue resolved, I think we're ready to merge this.

@cyrush cyrush merged commit f04d8fe into develop Jan 13, 2021
@cyrush cyrush deleted the task/2021_01_matset_to_silo_updates branch January 13, 2021 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants