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

pyCalculations/derivatives, VlsvWriter updates #158

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

alhom
Copy link
Contributor

@alhom alhom commented Sep 16, 2021

-VlsvWriter update for VisIt vlsv plugin compatibility
-VlsvReader utilities for reading, crude down- and upsampling, read_fg_var_as_volumetric
-pyCalculations/derivatives.py: Poynting flux and div thereof, curvature and ballooning criteria (w/ Jonas)

Copy link
Contributor

@markusbattarbee markusbattarbee left a comment

Choose a reason for hiding this comment

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

Interesting helper routines and quite a few of them. If there's now an effort of fleshing out this analysator fsgrid functionality, would you mind adding these as well?

def get_fsgrid_cellsize (and using it in these functions)
read_fsgrid_variable_cellid(self,name, cellids=-1,operator="pass")
where for each requested cellid, it finds the mean fsgrid value
get_cellid_at_fsgrid_index(i,j,k)

pyCalculations/derivatives.py Outdated Show resolved Hide resolved
pyCalculations/derivatives.py Outdated Show resolved Hide resolved
pyCalculations/derivatives.py Outdated Show resolved Hide resolved
pyCalculations/derivatives.py Outdated Show resolved Hide resolved
pyPlots/plot_helpers.py Outdated Show resolved Hide resolved
pyVlsv/vlsvreader.py Outdated Show resolved Hide resolved
pyVlsv/vlsvreader.py Outdated Show resolved Hide resolved
pyVlsv/vlsvreader.py Show resolved Hide resolved
pyVlsv/vlsvwriter.py Outdated Show resolved Hide resolved
Copy link
Contributor

@markusbattarbee markusbattarbee left a comment

Choose a reason for hiding this comment

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

Good extension of functionality (though a lot of helper functions which makes the files more cumbersome to read through). My main issue with this is that now we have helper functions for numerical operations and physical variables and criteria both in derivatives.py and plot_helpers.py. So now we have lots of near-duplicates, leading to bloating and potential conflicts in how different functions expect input.

I think it would be preferable to use the existing framework of expressions (where you give a funciton a dictionary of variables and it does with them what it needs to), and perhaps, for clarity, split mathematical helper functions into a separate file under pyCalculations (but not necessarily derivatives, what an odd name). Or even two files, one for 2D, one for 3D.

pyVlsv/vlsvreader.py Show resolved Hide resolved
@alhom
Copy link
Contributor Author

alhom commented Sep 28, 2021

Yes, it's a bit awkward to have these separate (and frankly, I wasn't really sure what would be the best approach to begin with - so this ballooning stuff got collated to derivatives.py), but there were a couple of reasons to have this bunch on its own for the time being:

  • reliance on fsgrid variables (and re-centering them!), which have been somewhat fiddly to work with
  • the expectation that the outputs are to be written back to a sidecar, instead of being only used as an expression in plotting (so, not relying on plot_helpers top-level variables, or 2D plane choices for output formatting, either)

I could see some (or even most) of these as reducers, even - at least in principle, the operators there could be extended with differential ops (though, calling those data_reducers_ is a bit much!). Thoughts?

@alhom
Copy link
Contributor Author

alhom commented Dec 16, 2022

This is still supplanted by an incoming PR to chop this up a bit.

@@ -1073,6 +1093,44 @@ def calcLocalSize(globalCells, ntasks, my_n):

return np.squeeze(orderedData)

def read_fg_variable_as_volumetric(self, name, centering=None, operator="pass"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented in master!

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.

3 participants