Re-Introduce EOS and Opacities reader

[Adnan-Ali-Ahmad]

Does what the title states. I was unsure where to add the ism_physics.py file, but I ended up adding it to the core. EOS and opacities can be gathered like in the old version, with osyris.core.get_eos(dataset, fname).
Resistivities reader will be added later in a seperate pull request.

[Adnan-Ali-Ahmad]

Build is failing because yapf and flake8 are contradicting each other. Flake8 wants (in get_eos function declaration):
"xHep_eos": None}):
but yapf wants:
"xHep_eos": None }):
(where "}):" is on a newline), so I'm not sure how to fix this in a way where they're both happy.

[nvaytet]

I haven't really looked at the code changes yet.

I have to say I have been pretty reluctant to add this in again, because it is very specific to one version of Ramses (ramses_ism), and there are a lot of hard-coded things in there (such as the names of the fields, offsets...), which completely depend on the structure of the eos and opacity files.

If someone decides to change these files in the future, we need to change everything here also, which doesn't sound like a good idea.

I think it would be better to ask around the different communities that are using Ramses, to see if different applications have similar requirements (e.g. they need to read metallicity tables, or something else). I seem to remember that metallicities are just written in the output files.

My preferred solution would actually to have the opacities and the resistivities written in the outputs as additional variables.
The current method was implemented to save disk space when running simulations.
However, we have, on more than one occasion, struggled with this method because, e.g., two different opacity tables were used: one to run the simulation, and a different one afterwards to plot the data.
And this led to many days of head scratching, trying to make sense of the results.

I can see that if we were to just put the opacities and resistivities in the outputs, you couldn't access/visualize them for old simulations that were run without writing them to file. However, I think the changes in here are relatively well self-contained. You are basically supplying a list of points in a 2d or 3d space, and ask the function to interpolate these points onto a regular mesh you read from file. Instead of making this part of osyris, we could make it into a small separate python package, e.g. ism_interpolator or something like that. You could then supply your simulation density and temperature to that and it would do the return an array of interpolated values?

[Adnan-Ali-Ahmad]

I understand your point of view. It does make sense that one would not want to create these methods that depend on one specific version of ramses, as it would make the codebase much harder for you to maintain. However, part of what drew me to osyris (or osiris as it was called back then) is its simplicity and the many useful features it has. The much better API you've created with this current version is what made me move to it (I hated having to deal with units and typing operations as strings), but some of the tools that I was accustomed to, such as the opacities and EOS readers for instance are missing. I've added them to my fork, but the more my fork deviates from your main branch the harder it is for me to maintain it, which is why I try to open pull requests from time to time to sync the two versions. What I would suggest is instead of creating a new package, maybe we can create a "miscellaneous" or "utilities" folder containing these kinds of useful methods for users. We could also add things like coordinate transforms for instance, or even the column density method you've mentioned on slack. The more tools osyris offers its users the better it becomes in my opinion.

[nvaytet]

maybe we can create a "miscellaneous" or "utilities" folder containing these kinds of useful methods for users

I don't really see how putting things in a different folder is going to help. That folder would still be part of the code base and would still have to be maintained.

We could also add things like coordinate transforms for instance, or even the column density method you've mentioned on slack

Not sure I understand what you mean by 'coordinate transform'. The column density thing is very different, it's general to all ramses simulations, not just ISM physics.

The more tools osyris offers its users the better it becomes in my opinion.

Not if it means that it becomes difficult to maintain and it then breaks all the time. If we had perfect test coverage for everything, I might change my mind, but we are so far from this.

Don't get me wrong, it's not that I don't welcome the contributions, quite the opposite, but in this case I feel like until we can come up with an interface or mechanism which is more general, and does not heavily depend on binary file structures (e.g. loading the opacity files with something like np.loadtxt would help, but those data files would be large in ascii format), I think we should refrain from adding them. It's always easier to add things later, than to remove things later, because you then break people's notebooks.

I don't really see it as a massive hinderance having to import a small additional python package to do the interpolations.

[nvaytet]

One additional thought is that these interpolations also require having those files containing the eos, opacities or the resistivities.
And as far as I know, the eos file for example is not public.
So we cannot ship these files as part of osyris (and they would add a lot of size bloat anyway), one would have to know where to find them.

[Adnan-Ali-Ahmad]

I don't really see how putting things in a different folder is going to help. That folder would still be part of the code base and would still have to be maintained.

Yes. What I meant was that I'd like to see these features despite their maintenance costs. The "utilities" folder can contain routines that are repetitive. For example, each time I work with osyris I need to compute the radius of cells before proceeding to plot radial profiles of physical quantities (temperature, density etc...). This can be simplified by having an osyris routine for it instead of having to copy/paste code or importing my own personal mini-package. By "coordinate transform", I meant passing from Cartesian to other coordinate systems, like cylindrical or spherical for example, where you can also specify your basis. These are things that can simplify a lot of people's notebooks.

In addition, we could also add cartopy projections of data. I realize this sounds a bit like scope creep, and maintaining the code base is daunting task, but I'm always happy to help (even though I'm much less experienced than you in python). At the end of the day, you can decide which parts of the code need priority maintenance, and which ones can be maintained later on.

One additional thought is that these interpolations also require having those files containing the eos, opacities or the resistivities.
And as far as I know, the eos file for example is not public.
So we cannot ship these files as part of osyris (and they would add a lot of size bloat anyway), one would have to know where to find them.

Maybe the opacities/eos reader is indeed a bit too specific to ramses_ism, but I'm sure there are other examples where people might want a feature that is specific to some other ramses version.

[nvaytet]

By "coordinate transform", I meant passing from Cartesian to other coordinate systems, like cylindrical or spherical for example, where you can also specify your basis. These are things that can simplify a lot of people's notebooks.

This sounds like a good idea, and it's generic to all simulations, so yes, we should do this 👍
We would probably need the option to provide a custom origin for the coordinate system.
Any good ideas for a name for such a module/function?
osyris.spatial.cartesian_to_spherical, osyris.spatial.cartesian_to_cylindrical?

In addition, we could also add cartopy projections of data

Do you have an example of an application for this? I'm trying to understand what you need it for.

[Adnan-Ali-Ahmad]

I was thinking of having a single function wrap up everything, something like osyris.spatial.get_components(dataset, system="spherical", origin = [x,y,z], basis=[ux,uy,uz]). Keep in mind that by "basis" I mean how the unit vectors are oriented with respect to the base cartesian grid (eg. [0,0,1] means that the sphere's northern axis is aligned with the "z" axis, maybe you can find a better name for this).
One thing we might want to consider is what the user actually wants transformed (eg. does he want the velocity components to be expressed in spherical coordinates?, the B field? xyz? all?) and how we handle this in the call to the function. A clean way of doing this would be to make the coordinate system an intrinsic part of the dataset class, so a user can change coordinate systems by doing dataset.transform(system="spherical", origin=[0,0,0], basis=[0,0,1]) and the routine transforms all coordinate-dependent quantities in the dataset (position & vector quantities). Although this might be a bit too much work, it has the benefit of reduced memory usage.

Do you have an example of an application for this? I'm trying to understand what you need it for.

Fig.6 of your 2018 paper is a great example of this. Although at the time you may have used basemap, it is now deprecated in favor of cartopy.

[nvaytet]

i'll move these points to a new issue/discussion

[nvaytet]

See #52 and #53