Replacing OMAS with IMASDD everywhere #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orso82
added a commit
to ProjectTorreyPines/IMASdd.jl
that referenced
this pull request
Mar 21, 2024
orso82
added a commit
to ProjectTorreyPines/IMAS.jl
that referenced
this pull request
Mar 21, 2024
|
try again ProjectTorreyPines/IMASdd.jl@6a6968a |
|
I tried again. This time the issue is that we are not setting dd. equilibrium.time before setting dd.equilibrium.vacuum_toroidal_field.b0 I've opened this issue JuliaFusion/EFIT.jl#5 which is required to be fixed in EFIT.jl before we can fix this in this repo. @orso82 @eldond Test environment: Error message: core_profile_extension: Error During Test at /Users/gupta/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:166
Got exception outside of a @test
Can't assign data to `equilibrium.vacuum_toroidal_field.b0` before ["equilibrium.time"]
Stacktrace:
[1] error(s::String)
@ Base ./error.jl:35
[2] setproperty!(ids::IMASDD.IDS, field::Symbol, v::Vector{Float64}; skip_non_coordinates::Bool, error_on_missing_coordinates::Bool)
@ IMASDD ~/Git/ProjectTorreyPines/IMASDD.jl/src/data.jl:466
[3] setproperty!
@ ~/Git/ProjectTorreyPines/IMASDD.jl/src/data.jl:454 [inlined]
[4] geqdsk_to_imas!(eqdsk_file::String, dd::IMASDD.dd{Float64}; time_index::Int64)
@ SD4SOLPS ~/Git/ProjectTorreyPines/SD4SOLPS.jl/src/SD4SOLPS.jl:89
[5] geqdsk_to_imas!
@ ~/Git/ProjectTorreyPines/SD4SOLPS.jl/src/SD4SOLPS.jl:71 [inlined]
[6] macro expansion
@ ~/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:187 [inlined]
[7] macro expansion
@ /Applications/Julia-1.9.app/Contents/Resources/julia/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined]
[8] top-level scope
@ ~/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:168
[9] include(mod::Module, _path::String)
@ Base ./Base.jl:457
[10] exec_options(opts::Base.JLOptions)
@ Base ./client.jl:307
[11] _start()
@ Base ./client.jl:522 |
Tested this but got following error: ``` (base) gupta@F-CJXNMY7L7 SD4SOLPS.jl % julia --project test/runtests.jl Test Summary: | Pass Total Time lightweight_utilities | 4 4 1.0s Test Summary: | Pass Total Time actuator | 2 2 1.7s core_profile_extension: Error During Test at /Users/gupta/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:166 Got exception outside of a @test UndefVarError: `gradient` not defined Stacktrace: [1] extrapolate_core(edge_rho::Vector{Float64}, edge_quantity::Vector{Float64}, rho_output::Vector{Float64}) @ SD4SOLPS ~/Git/ProjectTorreyPines/SD4SOLPS.jl/src/supersize_profile.jl:58 [2] macro expansion @ ~/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:171 [inlined] [3] macro expansion @ /Applications/Julia-1.9.app/Contents/Resources/julia/share/julia/stdlib/v1.9/Test/src/Test.jl:1498 [inlined] [4] top-level scope @ ~/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:168 [5] include(mod::Module, _path::String) @ Base ./Base.jl:457 [6] exec_options(opts::Base.JLOptions) @ Base ./client.jl:307 [7] _start() @ Base ./client.jl:522 Test Summary: | Error Total Time core_profile_extension | 1 1 0.5s ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken. in expression starting at /Users/gupta/Git/ProjectTorreyPines/SD4SOLPS.jl/test/runtests.jl:165 ``` For testing, following versions of the pacjages were used (these are the points where IMASDD was used instead of OMAS): GGDUtils: ProjectTorreyPines/IMASggd.jl@b11ad15 SOLPS2IMAS: ProjectTorreyPines/SOLPS2imas.jl@f843e6a The main issue is that OMAS.gradient() function is defined in https://github.com/ProjectTorreyPines/OMAS.jl/blob/master/src/math.jl but no such function exists in IMASDD.
Using set_time for sample files where description is not in the standard format. Changes were made in the code to use ismissing in places to conform to requirements of IMASDD. In record_regular_mesh!(), the deepcopy of each element was replaced with deepcopy of the entire grid subset to improve speed. The test runtests.jl was updated to reflect the changes. All tests pass now with following versions: JuliaFusion/EFIT.jl@8580ca8 ProjectTorreyPines/IMASdd.jl@6a6968a ProjectTorreyPines/IMASggd.jl@b11ad15 ProjectTorreyPines/SOLPS2imas.jl@f843e6a Note that EFIT version that was used has not been merged with its master branch yet at the time of this test. In conclusion, in future, OMAS can be removed now.
anchal-physics
force-pushed
the
omas_imas
branch
from
97b5cda
to
7c2ddd3
Compare
anchal-physics
force-pushed
the
omas_imas
branch
from
7c2ddd3
to
c7c3c1e
Compare
|
All tests passed with IMASDD. See 864319e . Now I've put back OMAS and all tests pass here as well. This PR is ready for merge. |
eldond
reviewed
Mar 25, 2024
|
|
||
| env_with_git_url u: | ||
| @echo "Pulling sample files using dvc" | ||
| -dvc pull | ||
| @echo "Creating Julia environment with the git urls without creating local clones" | ||
| @echo "Generating Project.toml and Manifest.toml" | ||
| julia --project=. -e 'using Pkg; Pkg.rm(["OMAS", "GGDUtils", "SOLPS2IMAS", "EFIT"]); Pkg.add(url="git@github.com:ProjectTorreyPines/OMAS.jl.git", rev="master"); Pkg.add(url="git@github.com:ProjectTorreyPines/GGDUtils.jl.git", rev="master"); Pkg.add(url="git@github.com:ProjectTorreyPines/SOLPS2IMAS.jl.git", rev="master"); Pkg.add(url="git@github.com:JuliaFusion/EFIT.jl.git", rev="master"); Pkg.instantiate()' | ||
| julia --project=. -e 'using Pkg; Pkg.rm(["OMAS", "IMASDD", "GGDUtils", "SOLPS2IMAS", "EFIT"]); Pkg.add(url="git@github.com:ProjectTorreyPines/OMAS.jl.git", rev="master"); Pkg.add(url="git@github.com:ProjectTorreyPines/IMASDD.jl.git", rev="master"); Pkg.add(url="git@github.com:ProjectTorreyPines/GGDUtils.jl.git", rev="master"); Pkg.add(url="git@github.com:ProjectTorreyPines/SOLPS2IMAS.jl.git", rev="master"); Pkg.add(url="git@github.com:JuliaFusion/EFIT.jl.git", rev="master"); Pkg.instantiate()' |
There was a problem hiding this comment.
We should remove OMAS here, too, right?
There was a problem hiding this comment.
No, right now, we need to keep OMAS
eldond
approved these changes
Mar 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested this but got following error:
For testing, following versions of the pacjages were used (these are the points where IMASDD was used instead of OMAS): GGDUtils: ProjectTorreyPines/IMASggd.jl@b11ad15 SOLPS2IMAS: ProjectTorreyPines/SOLPS2imas.jl@f843e6a
The main issue is that OMAS.gradient() function is defined in https://github.com/ProjectTorreyPines/OMAS.jl/blob/master/src/math.jl but no such function exists in IMASDD.
@orso82 Is there a function in IMASDD that can replace this gradient function or should I open a issue to get it there?