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 functions for interacting with IMAS data #16

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

Add functions for interacting with IMAS data #16

wants to merge 20 commits into from

Conversation

eldond
Copy link
Collaborator

@eldond eldond commented Aug 1, 2024

New features:

  • geqdsk2imas!() writes data from one or more GEQDSKFile instance to the IMAS schema
  • imas2geqdsk() reads equilibrium and wall data from an IMAS dd and outputs GEQDSKFile(s)

Updates:

  • The path is split from filenames when recording file into GEQDSKFile during io

Close #15

@eldond eldond self-assigned this Aug 1, 2024
@eldond eldond marked this pull request as ready for review August 2, 2024 17:22
@orso82
Copy link
Contributor

orso82 commented Aug 2, 2024

@eldond this looks good, though I have not tested it.

One thing you could try is to use the identify_cocos() method https://github.com/ProjectTorreyPines/CoordinateConventions.jl/blob/290e6b7e062cebcb4b07d422d39718271ad042d0/src/CoordinateConventions.jl#L304 since gEQDSKs can be in COCOS 1,3,5,7

There's an open issue about ProjectTorreyPines/CoordinateConventions.jl#1 but that should not affect you here

The other thing is that I have now made https://github.com/ProjectTorreyPines/IMAS.jl public. I much rather you use the IMAS.flux_surfaces(eqt) function to get the derived quantities in dd. The intention is to register IMAS.jl in the general registry, though we need to wait a couple of days for the packages that it depends on to be registered there.

@eldond
Copy link
Collaborator Author

eldond commented Aug 2, 2024

@orso82 I deleted derived quantities other than X-points, since EFIT.jl has an X-point finding function. If IMAS can do better at finding X-points, then maybe EFIT's X-point finder should be deleted and IMAS should be used for this instead. If EFIT should keep an X-point finding function, then I think it's okay for its results to be loaded into IMASdd.

I will look at identify_cocos

@eldond
Copy link
Collaborator Author

eldond commented Aug 5, 2024

@orso82 Okay, I think this is done. I'm using identify_cocos, I put in a bunch of tests, and I documented the functions.

Copy link
Contributor

@orso82 orso82 left a comment

Choose a reason for hiding this comment

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

mostly comments on stile and using IMAS(dd) time handling features

src/io.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
eldond and others added 3 commits August 8, 2024 08:22
- input list as bullets
- don't put "function" before everything in doc
- cut excessive generic text for geqdsk2imas
@anchal-physics
Copy link
Member

This is not related to this branch's main motive but it would be nice if this small fix can be piggy backed into it. At this line:

psi_int = Interpolations.LinearInterpolation((r_eq, z_eq), psi_eq)

Using LinearInterpolationsis deprecated and will be removed in future. This warning is printed every ime we use this function. Please just rename it to linear_interpolations. Thanks.

Copy link
Contributor

@orso82 orso82 left a comment

Choose a reason for hiding this comment

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

👍 some more comments, mostly removing unnecessary manual handling of time-dependent quantities. Also some tips about keyword arguments.

@bclyons12 could you please chime in about cocos_clockwise_phi ?

src/io.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
src/io.jl Outdated Show resolved Hide resolved
@eldond
Copy link
Collaborator Author

eldond commented Aug 9, 2024

Removing some of the time assignments / setup seems to have been problematic. @orso82 , the way I tried to resolve this previously may not have been well aligned with ideal IMAS usage. How would you recommend addressing this?

julia> include("test/runtests.jl")
Precompiling EFIT
  1 dependency successfully precompiled in 5 seconds. 123 already precompiled.
Test Summary: | Pass  Total  Time
find xpoint   |   20     20  9.7s
Got the expected exception
Test Summary: | Pass  Total  Time
parse header  |    4      4  0.1s
Test files are identified with COCOS = [5, 7]
test file 1 info: filename = g000001.01000, time = 0.001
imas: Error During Test at /home/eldond/.julia/dev/EFIT/test/runtests.jl:156
  Test threw exception
  Expression: eqt.time == (gs[1]).time
  equilibrium.time_slice[1].time is missing
  Stacktrace:
   [1] getproperty(ids::IMASdd.IDS, field::Symbol)
     @ IMASdd ~/.julia/dev/IMASdd/src/data.jl:213
   [2] macro expansion
     @ ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/Test/src/Test.jl:669 [inlined]
   [3] macro expansion
     @ ~/.julia/dev/EFIT/test/runtests.jl:156 [inlined]
   [4] macro expansion
     @ ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined]
   [5] top-level scope
     @ ~/.julia/dev/EFIT/test/runtests.jl:143
test file 2 info: filename = g184833.03600, time = 3.6
imas: Error During Test at /home/eldond/.julia/dev/EFIT/test/runtests.jl:142
  Got exception outside of a @test
  equilibrium.time_slice[1].time is missing
  Stacktrace:
    [1] getproperty
      @ ~/.julia/dev/IMASdd/src/data.jl:213 [inlined]
    [2] time_array_local(ids::IMASdd.IDSvector{T} where T<:IMASdd.IDSvectorTimeElement)
      @ IMASdd ~/.julia/dev/IMASdd/src/time.jl:120
    [3] resize!(ids::IMASdd.IDSvector{IMASdd.equilibrium__time_slice{Float64}}, time0::Float64)
      @ IMASdd ~/.julia/dev/IMASdd/src/data.jl:969
    [4] resize!(ids::IMASdd.IDSvector{IMASdd.equilibrium__time_slice{Float64}})
      @ IMASdd ~/.julia/dev/IMASdd/src/data.jl:965
    [5] geqdsk2imas!(g::GEQDSKFile, eq::IMASdd.equilibrium{Float64}; wall::Nothing, geqdsk_cocos::Int64, dd_cocos::Int64, cocos_clockwise_phi::Bool, add_derived::Bool)
      @ EFIT ~/.julia/dev/EFIT/src/io.jl:427
    [6] macro expansion
      @ ~/.julia/dev/EFIT/test/runtests.jl:161 [inlined]
    [7] macro expansion
      @ ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined]
    [8] top-level scope
      @ ~/.julia/dev/EFIT/test/runtests.jl:143
    [9] include(fname::String)
      @ Base.MainInclude ./client.jl:489
   [10] top-level scope
      @ REPL[2]:1
   [11] eval
      @ ./boot.jl:385 [inlined]
   [12] eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
      @ REPL ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/REPL/src/REPL.jl:150
   [13] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/REPL/src/REPL.jl:246
   [14] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
      @ REPL ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/REPL/src/REPL.jl:231
   [15] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
      @ REPL ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/REPL/src/REPL.jl:389
   [16] run_repl(repl::REPL.AbstractREPL, consumer::Any)
      @ REPL ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/REPL/src/REPL.jl:375
   [17] (::Base.var"#1013#1015"{Bool, Bool, Bool})(REPL::Module)
      @ Base ./client.jl:432
   [18] #invokelatest#2
      @ ./essentials.jl:892 [inlined]
   [19] invokelatest
      @ ./essentials.jl:889 [inlined]
   [20] run_main_repl(interactive::Bool, quiet::Bool, banner::Bool, history_file::Bool, color_set::Bool)
      @ Base ./client.jl:416
   [21] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:333
   [22] _start()
      @ Base ./client.jl:552
Test Summary: | Pass  Error  Total  Time
imas          |    1      2      3  6.9s
ERROR: LoadError: Some tests did not pass: 1 passed, 0 failed, 2 errored, 0 broken.
in expression starting at /home/eldond/.julia/dev/EFIT/test/runtests.jl:142

@eldond
Copy link
Collaborator Author

eldond commented Oct 9, 2024

@orso82 whoops, this got forgotten. Currently stuck on trying to use IMAS time handling as intended.

@orso82
Copy link
Contributor

orso82 commented Oct 12, 2024

let me know if you need help @eldond

@orso82
Copy link
Contributor

orso82 commented Oct 23, 2024

@eldong FYI that @mgyoo86 is interested in this

@orso82 orso82 mentioned this pull request Nov 14, 2024
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.

Utility for writing to IMAS
4 participants