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 spacing issue for a network of two pores #1844

Merged
merged 4 commits into from
Feb 13, 2021
Merged

Fix spacing issue for a network of two pores #1844

merged 4 commits into from
Feb 13, 2021

Conversation

Ni2M
Copy link
Contributor

@Ni2M Ni2M commented Feb 11, 2021

This PR closes #1732

Sample code:

import openpnm as op
import numpy as np
net = op.network.Cubic([1, 3, 1])
print(net.spacing)

output:

[0. 1. 0.]
# net = op.network.Cubic([3, 3, 3]) # still works for 3D networks
# [1. 1. 1.]
# net = op.network.Cubic([3, 3, 1]) # still works for 2D networks
# [1. 1. 0.]

@Ni2M Ni2M self-assigned this Feb 11, 2021
@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #1844 (f7ac6ba) into dev (11530fa) will decrease coverage by 0.0%.
The diff coverage is 50.0%.

@@           Coverage Diff           @@
##             dev   #1844     +/-   ##
=======================================
- Coverage   87.1%   87.1%   -0.1%     
=======================================
  Files        147     147             
  Lines      12645   12644      -1     
=======================================
- Hits       11020   11019      -1     
  Misses      1625    1625             

Copy link
Member

@ma-sadeghi ma-sadeghi left a comment

Choose a reason for hiding this comment

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

Great, could you also add a unit test to catch the corner cases? (the line with raise Exception, and the lines after else clause.

Also, could you try it with networks with only two pores? In your example, you have 3 pores. Try shape = [1, 2, 1] or [2, 1, 1]

@Ni2M
Copy link
Contributor Author

Ni2M commented Feb 12, 2021

Thank you Amin for review and comments. For sure. I will implement them.

@Ni2M
Copy link
Contributor Author

Ni2M commented Feb 12, 2021

After reviewing the code, I think we have two raise exception that are related. To raise an error in the first exception the throat must not be aligned with the lattice (directional throat) which can happen by changing the pore coords "test_spacing_on_joggled_network". This exception also occurs when we add boundary pores (which results in len(c.keys()) >sum(dims)) unit test of "test_spacing_on_network_with_boundary_pores". Both of these cases fall in the category of raise error later in the code ("A unique value of spacing could not be found"), but they are already captured ealier in with the first exception. If we try following code, they will be captured by second exception but only because the first exception is commented.

import openpnm as op
import numpy as np
from openpnm import topotools
net = op.network.Cubic([2, 2, 1])
#net.add_boundary_pores()
print(net['pore.coords'])
P12 = net["throat.conns"]
net['pore.coords'][2]=[1.2,0.5,0.5]# instead of [1.5,0.5,0.5]
C12 = net["pore.coords"][P12]
mag = np.linalg.norm(np.diff(C12, axis=1), axis=2)
unit_vec = np.around(np.squeeze(np.diff(C12, axis=1)) / mag, decimals=14)
spacing = [0, 0, 0]
dims = topotools.dimensionality(net)
# Ensure vectors point in n-dims unique directions
# c = {tuple(row): 1 for row in unit_vec}
# if len(c.keys()) >sum(dims):
#     raise Exception(
#         "Spacing is undefined when throats point in "
#         + "more directions than network has dimensions"
#     )
mag = np.float64(mag.squeeze())
for ax in [0, 1, 2]:
    if dims[ax]:
        inds = np.where(unit_vec[:, ax] == unit_vec[:, ax].max())[0]
        if np.ndim(mag) != 0:
            temp = np.unique(mag[inds])
            if not np.allclose(temp, temp[0]):
                raise Exception("A unique value of spacing could not be found")
            spacing[ax] = temp[0]
        else:
            temp = mag
            spacing[ax] = temp
print(spacing)

In other words, I could not find any other case that does not raise error in the first exception but still raises error in the second exception. Because the condition of unit_vec will be checked in the first exception already. I would appreciate any comment related to this problem. Thank you.

@Ni2M
Copy link
Contributor Author

Ni2M commented Feb 12, 2021

Current results to check 2 pores matches:

import openpnm as op
import numpy as np
#net = op.network.Cubic([1, 2, 1])
net = op.network.Cubic([2, 1, 1])
print(net.spacing)

#[0. 1. 0.]
#[1. 0. 0.]

@ma-sadeghi ma-sadeghi merged commit e1d10b6 into dev Feb 13, 2021
@ma-sadeghi ma-sadeghi deleted the spacing branch February 17, 2021 00:39
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.

network.spacing breaks for 1D networks with only two pores
2 participants