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

Make EFITequilibrium class variables public #349

Closed
munechika-koyo opened this issue Dec 10, 2021 · 13 comments
Closed

Make EFITequilibrium class variables public #349

munechika-koyo opened this issue Dec 10, 2021 · 13 comments

Comments

@munechika-koyo
Copy link
Contributor

As mentioned in PR #348, I threw the issue about it.
Why I proposed this issue is that I need to reconstruct some function classes (e.g. MagneticField) and apply them to EFITequilibrium variables to use my own tokamak device and calculated equilibrium information.

@CnlPepper
Copy link
Member

Could you expand on what it is you are attempting to do with the EFITEquilibrium class? This class is meant to be immutable as it is representing the state of an EFIT run. If it is being used to pass data around, rather than passing the functions held on its attributes then it is being used incorrectly.

@munechika-koyo
Copy link
Contributor Author

I actually apply the results of Tokamak Simulation Code (TSC) to EFITequilibrium class to use the useful methods implemented in it like this example. At first, I just instantiated it by adding some arguments but I decided to inherit it because it is needed to apply re-defined classes such as a MagneticField, which is not assumed in terms of using not EFIT results but others.

@munechika-koyo
Copy link
Contributor Author

I should have defined the original one, but the methods implemented on EFITequilibrium class such as map3d or 2d is so useful that it has allowed me to facilitate to create a plasma object, plot some profiles, etc.

@Mateasek
Copy link
Member

Hi @munechika-koyo , I'm sorry but I still don't understand the issue. Could you please help me a bit more? Is it that you would like to be able to define the equilibrium with a different set of parameters?

@munechika-koyo
Copy link
Contributor Author

Thank you for your reply @Mateasek!
Roughly speaking, yes it is.
However, I could not manage to generate a new equilibrium only with different parameters which are offered in EFITEquilibrium to obtain the magnetic information such as a b_field.
So I needed to implement my own MagneticField class and apply it to b_field variables in EFITEquilibrium.

@Mateasek
Copy link
Member

If you have your magnetic field in 3D, you should be able to calculate most of the values the EFITEquilibrium needs to be initialised, shouldn't you?

@munechika-koyo
Copy link
Contributor Author

Actually, I suppose so.
I defined my own class because b_vacuum_radius and b_vacuum_magnitude values were required to calculate the magnetic field outside LCFS, which were not offered by TSC. It produces the current flux profile (f profile) including outside LCFS, but probably I would be able to prepare b_vacuum_magnitude using an f profile raw data at a certain (r, z) point outside lcfs.

@vsnever
Copy link
Member

vsnever commented Dec 14, 2021

If the problem is only with b_field, we can make it a property as a hotfix. Indeed, the MagneticField is not very accurate outside LCFS. More general equilibrium classes will still be needed in the future.

@CnlPepper
Copy link
Member

The EFITEquilibrium is for EFIT data. Don't break it's abstractions to support another data source. The correct approach would be to pull all the common functionality into a base class (or set of base classes) and derive a new equilibrium object from that, clean base.

If you solve problems by breaking abstractions you will destroy the protections cherab provides to reduce the risk of unexpected behaviours. Do not do it!

@CnlPepper
Copy link
Member

CnlPepper commented Dec 14, 2021

Matt and I always intended for the equilibrium tools to be expanded. The EFITEquilibrium was just the first step. The next stages were, as @Mateasek has suggested, to generalise the code. 2D and 3D Equilibrium objects should provide a common interface and subclasses ahould be provided for each data source. The subclasses would likely contain additional attributes/methods to cover the "extra" data specific to each source.

@munechika-koyo
Copy link
Contributor Author

I agree to the @CnlPepper's opinion.
I also intend to use cherab's classes like EFITEquilibrium to apply to completely 3D magnetic configurations such as stellarators.
So more common base class is preferable for me.

@Mateasek
Copy link
Member

For now I would really suggest @munechika-koyo to make function, which calculates all the needed parameters for EFITEquilibrium, from the parameters you have and returns the EFITEquilibrium object, if this is what you need. I did this several times, when there was something missing/defined through different physical quantity.

I'm not an equilibrium expert, so maybe there can be a different path. But I thought the first thing we could do is to define a set of functions, which calculate equilibrium quantities from other equilibrium inputs and other data. This is given by physics and imo is the most general thing. Then, we can start defining equilibrium objects, which would be a sort of a data and function containers, using the defined functions. As far as I know, there are more ways how to get a working equilibrium class, with certain input data. Some may be more numerically accurate than other. This approach may give us and users all the needed building blocks.

@munechika-koyo
Copy link
Contributor Author

Thank you @Mateasek
I would follow your recommendation firstly.

@jacklovell jacklovell closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2023
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 a pull request may close this issue.

5 participants