modflowpy / flopy

A Python package to create, run, and post-process MODFLOW-based models.
https://flopy.readthedocs.io
Other
507 stars 307 forks source link

bug: weird logic for checking "Size of vector" in add_transient_vector() #2210

Open mickey-tsai opened 3 months ago

mickey-tsai commented 3 months ago

Describe the bug

https://github.com/modflowpy/flopy/blob/develop/flopy/export/vtk.py#L969-L975

I am not a modflow professional. The assertion error "Size of vector must be 3 nnodes or 3 ncpl" is triggered when value.size != 3 self.ncpl or value.size != 3 self.nnodes. This assertion error is always triggered in most normal cases.

Expected behavior To my understanding, nnodes =layer*ncpl. Should the condition "layer =1" be considered?

if value.size == 3 * self.ncpl:
  //do something
elif value.size == 3 * self.nnodes:
  //do something
elif layer == 1:
  //do something
else:
  raise Exception()
wpbonelli commented 3 months ago

Hi @mickey-tsai

nnodes is the total number of nodes in the grid. This is defined differently for each grid type

I think you are right, something is funny here. At very least, we need to switch or -> and

- value.size != 3 * self.ncpl or value.size != 3 * self.nnodes
+ value.size != 3 * self.ncpl and value.size != 3 * self.nnodes

... or pull the negation out front per de morgan.

But I'm not clear on the intent behind this code, nor do we have test cases or example models using add_transient_vector() in context, maybe @jlarsen-usgs can come to the rescue

jlarsen-usgs commented 2 months ago

@mickey-tsai

I'll have to look a little more into how and why I developed the vector a little more. It's been awhile.

My guess is that since the VTK file is in three dimensional space the associated vector would need to have three components. An example of this is Specific Discharge which has the components [qx, qy, qz].