Skip to content

Conversation

@bam241
Copy link
Contributor

@bam241 bam241 commented Jan 15, 2025

Description

This is a follow up on svalinn/DAGMC#805

This ensures that a assignment named "not_a_vacuum" is not flagged as vacuum and therefore is not assigned with vacuum boundary condition.

Fixes # (issue)
This is related to #3218

I also had to implement that a different way than initially expected.
I first thought about leveraging the #3056 PR recently merged to do this kind of test.
But, vacuum detection is perform (in the DAGMC workflow within openMC) before applying materials assignment (therefore before overriding)

This is why I implemented this by making temporary copy of the dagmc.hm5 and modifying them. (Not that seing how simple this is we could potentially provide a method that allows to do that within dagmc.py ?)

I trying and failed to design this test as a fixture, I don't know why it failed it never recognised the returned model as a openmc.Model but as a function... (@pshriwise if you have any insight on this ?)

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

This works is sponsored by Proxima Fusion

@bam241 bam241 requested a review from pshriwise as a code owner January 15, 2025 09:58
@bam241 bam241 changed the title Vaccum detction test Ensuring only exact "vacuum" assignment is detected as vacuum Jan 15, 2025
Copy link
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

@bam241 I took a crack at simplifying this test somewhat. I'm pretty sure it still tests what you're intending, but let me know if I'm missing something.

I don't have a suggestion on how to re-use the model fixture above here. I don't think it's too important though as the materials in this test could be anything as long as their names are set appropriately.

@bam241
Copy link
Contributor Author

bam241 commented Jan 23, 2025

yes it does !
I didn't know I could check presence of a material like that, it is great.
I'll add some more assert to exactly match what I wanted to test, but thx for the suggestion !

@pshriwise
Copy link
Contributor

Sweet! I'll keep an eye out for your updates!

@bam241
Copy link
Contributor Author

bam241 commented Apr 12, 2025

@pshriwise I finally got the time to get back to it.

The goal here, is to make sure, a material assignment named "vacuum" is empty and one named "not_a_vacuum" is properly defined.
(this is a follow-up test to ensure the former DAGMC behaviour flagging any assignment with vacuum in it as vacuum)

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.

2 participants