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

add IO functionalities to write DCIP2D mesh and models #84

Merged
merged 18 commits into from
Jan 8, 2018

Conversation

thast
Copy link
Member

@thast thast commented Jan 3, 2018

  • the MeshIO functions writeUBC and writeModelUBC are now compatible with 2D meshes and models.

  • We now make sure that mesh and model get saved at the same place by giving the option to specify the folder outside of the filename (before the folder was given by the dict.keys, not very practical)

  • The upgrade is backward compatible

@codecov
Copy link

codecov bot commented Jan 3, 2018

Codecov Report

Merging #84 into master will increase coverage by 0.38%.
The diff coverage is 91.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   77.06%   77.44%   +0.38%     
==========================================
  Files          17       17              
  Lines        4870     4948      +78     
==========================================
+ Hits         3753     3832      +79     
+ Misses       1117     1116       -1
Impacted Files Coverage Δ
discretize/MeshIO.py 85.43% <91.07%> (+1.87%) ⬆️
discretize/utils/meshutils.py 75% <0%> (+5.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c876f65...5be5348. Read the comment docs.

Copy link
Member

@lheagy lheagy left a comment

Choose a reason for hiding this comment

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

Thanks for this! Just a couple minor comments. I am happy to upload the testing files so that we don't need to keep track of them in git. Feel free to send them my way via slack and I will pass you back a url

@@ -300,7 +300,56 @@ def _toVTRObj(mesh, models=None):
vtkObj.GetCellData().SetActiveScalars(models.keys()[0])
return vtkObj

def readModelUBC(mesh, fileName):
def _readModelUBC_2D(mesh, fileName):
"""
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this @thast!


else:

if len(obsfile[1:]) == 1:
Copy link
Member

Choose a reason for hiding this comment

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

would you mind adding a test that hits these lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

we can actually make this function much simpler. Will do.

@@ -0,0 +1,48 @@
135 47
Copy link
Member

Choose a reason for hiding this comment

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

@thast: is this for testing, if you slack me the file, I will upload and send you a url where you can download it so it is not in the core repo

Copy link
Member Author

Choose a reason for hiding this comment

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

@lheagy : Thanks for pointing it out. The test actually writes and then read back the testing files here (so we test both input and output).
I will reproduce what we do in SimPEG and create a temporary folder that we then delete, so no storing will be necessary.

3550 1
3950 1
4550 1
!comment line
Copy link
Member

Choose a reason for hiding this comment

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

same here, I am happy to upload this

@@ -118,7 +118,7 @@ def unpackdx(fid, nrows):
return tensMsh

@classmethod
def readUBC(TensorMesh, fileName, meshdim=None):
def readUBC(TensorMesh, fileName, meshdim=None, folder=''):
Copy link
Member

Choose a reason for hiding this comment

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

do we want folder or directory ?
cc @rowanc1: do you know what is more common?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed directory does sound better.

# Fist line is the size of the model
sizeM = np.array(msh.ravel()[0].split(), dtype=float)
# Check if the mesh is a UBC 2D mesh
if sizeM.shape[0] == 1:
Tnsmsh = TensorMesh._readUBC_2DMesh(fileName)
Tnsmsh = TensorMesh._readUBC_2DMesh(fname)
Copy link
Member

Choose a reason for hiding this comment

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

to simplify this a bit (and reduce redundancy), why don't we just assign meshdim = 2 here or meshdim = 3 below, and then check meshdim and call the appropriate readUBC method after that.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, this is a function I wrote in a previous PR but it was a redundant check between what the user expects and what the machine read.

Copy link
Member

@lheagy lheagy left a comment

Choose a reason for hiding this comment

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

Thanks for all of this @thast! I left one more minor comment to simplify the code a bit. Once that is done, I think this is good to go!
@rowanc1: do you want to take a look at this?

Copy link
Member

@lheagy lheagy left a comment

Choose a reason for hiding this comment

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

Thanks @thast, this looks great!

@thast
Copy link
Member Author

thast commented Jan 7, 2018

Thanks @lheagy, can you merge it for me please? Does not seem I have the rights to do so
("Only those with write access to this repository can merge pull requests.")

@lheagy lheagy merged commit 5d802e3 into master Jan 8, 2018
@lheagy lheagy deleted the write_ubc_2d_mesh_Thibaut branch January 8, 2018 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants