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

Final switch to IMASDD #26

Merged
merged 4 commits into from
Apr 2, 2024
Merged

Final switch to IMASDD #26

merged 4 commits into from
Apr 2, 2024

Conversation

anchal-physics
Copy link
Collaborator

@anchal-physics anchal-physics commented Mar 29, 2024

This PR would be merged simultaneously when all other repos also have the PR ready for the final switch from OMAS to IMASDD.

Speed test

I ran a speed test to check the difference between using OMAS and IMASDD. With IMASDD, add_inteferometer is 10 times slower. Interestingly, add_langmuir_probe is marginally faster. We'll need to find bottleneck in this repo as well. Following are the results:

Speed test with IMASDD.jl

SynthDiag.jl % julia --project test/speedtest.jl
-----------------------------------------------------------------------------
┌ Warning: equilibrium.vacuum_toroidal_field.b0 was skipped in dict2imas: ErrorException("Can't assign data to `equilibrium.vacuum_toroidal_field.b0` before [\"equilibrium.time\"]")
└ @ IMASDD ~/Git/ProjectTorreyPines/IMASDD.jl/src/io.jl:194
┌ Warning: equilibrium.vacuum_toroidal_field.b0 was skipped in dict2imas: ErrorException("Can't assign data to `equilibrium.vacuum_toroidal_field.b0` before [\"equilibrium.time\"]")
└ @ IMASDD ~/Git/ProjectTorreyPines/IMASDD.jl/src/io.jl:194
add_interferometer!() time with compilation:  46.538955 seconds (157.29 M allocations: 3.633 GiB, 1.37% gc time, 10.17% compilation time)
add_interferometer!() time (true runtime):  42.258029 seconds (142.16 M allocations: 2.637 GiB, 0.63% gc time)
-----------------------------------------------------------------------------
add_langmuir_probes!() time with compilation:   3.012272 seconds (7.34 M allocations: 694.927 MiB, 2.89% gc time, 63.21% compilation time)
add_langmuir_probes!() time (true runtime):   0.993887 seconds (220.37 k allocations: 196.591 MiB)

Note that the warning is due to outdated sample file which is being read by IMASDD. I'll fix this sample file soon.

Speed test with OMAS.jl

SynthDiag.jl % julia --project test/speedtest.jl 
-----------------------------------------------------------------------------
add_interferometer!() time with compilation:   4.682815 seconds (703.33 k allocations: 571.787 MiB, 1.96% gc time, 12.18% compilation time)
add_interferometer!() time (true runtime):   4.513775 seconds (102.93 k allocations: 532.390 MiB, 0.57% gc time)
-----------------------------------------------------------------------------
add_langmuir_probes!() time with compilation:   1.645637 seconds (1.84 M allocations: 307.415 MiB, 1.41% gc time, 30.44% compilation time)
add_langmuir_probes!() time (true runtime):   1.135085 seconds (190.72 k allocations: 195.935 MiB)

@anchal-physics
Copy link
Collaborator Author

Inteferometer operation has been sped up by optimizing \in operation for point in a subset in GGDUtils at ProjectTorreyPines/IMASggd.jl@392b428

Now speed test results are:

SynthDiag.jl % julia --project test/speedtest.jl
-----------------------------------------------------------------------------
add_interferometer!() time with compilation:   9.951131 seconds (50.11 M allocations: 4.110 GiB, 5.75% gc time, 48.21% compilation time)
add_interferometer!() time (true runtime):   5.389749 seconds (35.12 M allocations: 3.124 GiB, 4.63% gc time)
-----------------------------------------------------------------------------
add_langmuir_probes!() time with compilation:   3.002231 seconds (7.34 M allocations: 694.869 MiB, 2.85% gc time, 62.67% compilation time)
add_langmuir_probes!() time (true runtime):   0.983486 seconds (220.36 k allocations: 196.590 MiB)

This is almost the same performance as when OMAS was used. So now we can consider this PR ready for merge as well.

@anchal-physics
Copy link
Collaborator Author

I'll wait for #25 to be accepted before submitting this PR for review. I'll rebase with dev and see if gas_injection also requires any optimization with IMASDD.

* All references to OMAS have been removed now.
* Some changes in interferometer.jl were lost in previous PR merges, so I have added them again.
* All tests pass with IMASDD.
@anchal-physics
Copy link
Collaborator Author

I have rebased to incorporate #25 . A speedtest has been added for gas_injection functions too and it looks fine.

SynthDiag.jl % julia --project test/speedtest.jl
-----------------------------------------------------------------------------
add_interferometer!() time with compilation:   9.925406 seconds (50.21 M allocations: 4.118 GiB, 5.98% gc time, 48.09% compilation time)
add_interferometer!() time (true runtime):   5.916004 seconds (35.12 M allocations: 3.124 GiB, 4.24% gc time)
-----------------------------------------------------------------------------
add_langmuir_probes!() time with compilation:   2.851358 seconds (7.11 M allocations: 680.428 MiB, 2.67% gc time, 67.77% compilation time)
add_langmuir_probes!() time (true runtime):   0.951874 seconds (220.36 k allocations: 196.590 MiB)
-----------------------------------------------------------------------------
get_gas_injection_response() time with compilation:  14.438887 seconds (4.16 M allocations: 42.286 GiB, 30.00% gc time, 1.47% compilation time)
get_gas_injection_response() time (true runtime):  13.658842 seconds (3.71 M allocations: 42.256 GiB, 28.88% gc time)
compute_gas_injection() time with compilation:   1.159418 seconds (3.57 M allocations: 229.790 MiB, 99.73% compilation time)
compute_gas_injection() time (true runtime):   0.001134 seconds (30.55 k allocations: 1.314 MiB)

This PR is ready for merge now. I'll submit all four PRs for review.

@anchal-physics anchal-physics changed the title WIP: Final switch to IMASDD Final switch to IMASDD Apr 1, 2024
@anchal-physics anchal-physics merged commit 2c8f53f into dev Apr 2, 2024
1 check passed
@eldond eldond deleted the omas_imas branch April 2, 2024 16:34
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.

2 participants