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

Expose set locations #1705

Merged
merged 13 commits into from
Nov 5, 2020
Merged

Expose set locations #1705

merged 13 commits into from
Nov 5, 2020

Conversation

jgostick
Copy link
Member

This PR focuses on providing the user with more power to change physics and geometry associations.

The hidden _set_locations method on the Subdomain class now has two public wrappers on the GenericGeometry class, add_locations and drop_locations. These both call the _set_locations method, and do a bit of extra clean-up. Crucially, these methods will also change the locations of any associated physics objects. If I remember correctly, we had hidden these methods because we didn't want users to start fiddling with locations on a geometry, since this would break the associated physics. So we made it difficult to change these associations after the objects were made. This new approach addresses this, since it keeps things correctly connected.

This also adds set_phase and set_geometry method to the GenericPhysics class. Changing phase is straightfoward. Changing geometry effectively changes which pores and throats the physics is associated with. Each of the methods has several modes, including 'add', 'drop', and 'swap'.

Unit tests were added for all new methods.

@codecov
Copy link

codecov bot commented Oct 18, 2020

Codecov Report

Merging #1705 into dev will increase coverage by 0.0%.
The diff coverage is 97.3%.

@@          Coverage Diff          @@
##             dev   #1705   +/-   ##
=====================================
  Coverage   86.7%   86.7%           
=====================================
  Files        147     147           
  Lines      12387   12424   +37     
=====================================
+ Hits       10748   10783   +35     
- Misses      1639    1641    +2     

@jgostick
Copy link
Member Author

This PR will need a bit of discussion. I basically doubles-down on the use of Geometry objects to define where Physics object apply. In principle, a single physics object can be applied to an entire domain; however, lots of 'look-ups' depend on a 1-to-1 correspondence between geometries and physics. Note that this current PR is 100% backwards compatible with our tests. We could, alternatively, have reverted back to the style of version 1, where physics are assigned by specific pores and throats. I am not sure if that would have been backwards-compatible or not. I can say that the data doesn't really care if physics have a 1-to-1 match with geometries (ie. interleave data, interpolate data, models, etc would all work). It's just the object look-ups at the project level that have a problem.

@jgostick
Copy link
Member Author

Fixes #1407

@jgostick
Copy link
Member Author

This is now ready to go. I fixed the bugs we encountered during yesterday's meeting and also fixed a bug in the grid.

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.

1 participant