Update ec_launchers and pulse_schedule

[olivhoenen]

---

📚 Documentation preview 📚: https://imas-data-dictionary--253.org.readthedocs.build/en/253/

[github-actions]

Diff of generated Data Dictionary XML file

[olivhoenen]

This PR has merged changes from PR#234 and PR#244, in order that @fmpoli68 can test this change in applications. It stays in draft for the moment.

[fmpoli68]

We have started the tests for this branch.

Below are the issues we found.

In the ec_launchers IDS

1. waveguide
   │ ├── waveguide(i1)/name
   │ ├── waveguide(i1)/description
   │ ├── waveguide(i1)/geometry
   │ │ └──
   │ └── waveguide(i1)/movement(i2)
   │ └──

movement should be removed, since waveguides are fixed.

2. I lost track of why we removed the mirror_index under beam.
   => please put it back
   It is not clear how to best represent a complex optical path. We need to test with the mirror_index
   ├── beam(i1)/waveguide_index
   │ ├── beam(i1)/source_index
   │ ├── beam(i1)/mirror_index

In the pulse_schedule IDS:

NOTE: I am not sure how many people are using the pulse_schedule. To avoid disruption to these users I propose to create the correct structures, but not delete the existing structures that are incorrectly described (for now).
For example, the following do not belong to the pulse_schedule/ec/beam:
beam/deposition_rho_tor_norm
beam/steering_angle_toroidal
beam/steering_angle_poloidal

Change requested:

1. please, add deposition_rho_tor_norm under mirror.
   The description should be replaced with the following: "Normalized toroidal flux coordinate at which the main deposition should occur. For tracking a rational surface use a negative value (for example -1.5 or -2.0)".

2. the polarization should be set in the pulse_schedule (O_mode, X_mode).
   Can we copy here the polarization structure from the ec_launchers
   pulse/ec/beam/polarization
   After testing, we will be able to tell whether we need the full structure. Perhaps here we need only O_mode_fraction, but we will see.

@imbeauf If the above can be implemented in a short time workaround, we can do a number of tests, including algorithms for steering. Then we will come back with a proposal to map which gyrotron is connected where in the pulse_schedule.

[imbeauf]

For the ec_launcher IDS

1. I don't understand your comment, because in this branch the structure you describe in 1) is correctly located under the "mirror" AoS, not under the "waveguide" AoS
2. Now we have mirror_indices under waveguide, since I understood that the optical path is defined by one waveguide and a set of mirrors. If you add a mirror_index under beam, which mirror is it (since there may be several mirrors or 0 in the optical path) ? Isn't it better like this to have the waveguide_index under beam, then you know the full optical path followed by the beam from waveguide/mirror_indices ?
   I think you expect to have possibly multiple beams from the same optical path, correct ?

[imbeauf]

I have implemented in my branch the changes requested for the pulse_schedule IDS, I am waiting for your response on the ec_launchers part to push my branch

[fmpoli68]

regarding 1)

• the imas.util.print_metadata_tree shows "movement" under waveguide (not sure why), but you are right that the documentation preview does not show that.

regarding 2)
After some thinking and deeper discussion with Lorenzo Figini, below are the requested changes in ec_launchers.

• source_index should be moved from beam to waveguire. It should not be under beam.
• beam/mirror_index. This is needed so that codes verify that everything is calculated correctly in the optical path (the beam/mirror_index must correspond to the last element of waveguide/mirror_indices). Can I suggest that we add it, We write/test all our scripts for the optical path (which will be released as open source), and then propose a final design? Which might include remove the mirror_index again.

If you push your changes, we will proceed with further tests and get back to you ASAP (likely before Wednesday).

[github-actions]

Diff of generated Data Dictionary XML file

[imbeauf]

Done, please test the branch with the updated pulse_schedule

[fmpoli68]

Not sure you saw these (I do not see these changes in the latest push):

in the ec_launchers IDS

• source_index should be moved from beam to waveguide. It should not be under beam.
• beam/mirror_index. This is needed so that codes verify that everything is calculated correctly in the optical path (the beam/mirror_index must correspond to the last element of waveguide/mirror_indices). Can I suggest that we add it back, at least to support our development/tests of the codes for the optical path? At the end of the exercise we might decide that it is redundant, but we need this information in phase of development.

I will test the new pulse_schedule in the meantime. Thank you for doing this :).

Thank you for your patience

[github-actions]

Diff of generated Data Dictionary XML file

[imbeauf]

Done

[fmpoli68]

We have tested the new IDS structure with a few standalone scripts and we think that everything we need to proceed is there.
Other tests (in complex workflows) will have to wait until this branch is merged into develop (some conflicts between DD versions).
Please, proceed with the pull request and merging.

And many many thanks for helping out.

[github-actions]

Diff of generated Data Dictionary XML file

[imbeauf]

Just one question before merging: we have the power_launched at the beam level, but we have no power at the source level (just a power_max node as a function of the available frequencies). Is this ok to have no power at the source level ?

[fmpoli68]

Yes, it is correct. The power_max is the power that is available from that source (for example, a gyrotron). In a complete calculation from the source to the plasma (from the source to the beam) one may want to include losses (in the waveguide, for example), or from losses along the path. Therefore, the power at the launching point of the beam may be different from the nominal power that a source can deliver, and the power at the beam level can be dynamic, while the power at the source level is nominal.

Regarding the frequency, it is also OK to have a static number at the source level, because sources are optimized to work at one frequency. So, the cavity resonates at one frequency.

I think this is the closest representation we can have at this time for the system.
We might come back with additional request as we expand our applications.

[github-actions]

Diff of generated Data Dictionary XML file

[imbeauf]

Dear reviewers, please have a look to this PR, which is ready to be reviewed for a few weeks now

[SimonPinches]

Why use toroidal flux rather than poloidal flux which is defined everywhere? If I understand correctly, if the number is negative then the value of the safety factor is specified instead. What happens then when the safety factor is negative as in AUG? (Or, if you prefer, if m/n is negative?). I guess we could assume -abs(q) = -abs(m/n)?

[imbeauf]

@fmpoli68 , could you comment on this proposal ?

[SimonPinches]

Suggested change

	<int name="elliptical" description="Prolate ellipsoid (surface obtained rotating an ellipse around its major axis). Surface defined by focus_1, focus_2 and geometry/centre (point on the surface) coordinates" >6</int>
	<int name="elliptical" description="Prolate ellipsoid (surface obtained by rotating an ellipse around its major axis). Surface defined by focus_1, focus_2 and geometry/centre (point on the surface) coordinates" >6</int>

Rather that geometry/centre, don't we need the minor radius to define a prolate ellipsoid? If the point on the surface was at a pole (on major axis) then this would tell us nothing about the minor radius of the ellipsoid.

[imbeauf]

@fmpoli68 , could you comment on this proposal ?

[fmpoli68]

NOTE: I used Copilot to support my answer with formulas.

The definition is mathematically sufficient, we do not need the minor radius, because the definition does not use:

centre + surface point

It uses:

focus_1
focus_2
a point on the surface (despite the confusing label geometry/centre)

For a prolate spheroid:

The two foci determine the axis direction and the focal distance ccc.
Any point on the surface gives 2a=∣P−F1∣+∣P−F2∣,2a = |P-F_1| + |P-F_2|,2a=∣P−F1​∣+∣P−F2​∣, from which aaa is obtained.
The minor radius then follows uniquely from b=a2−c2.b = \sqrt{a^2-c^2}.b=a2−c2​.

Even if the surface point lies exactly at a pole, we still recover aaa, and since ccc is known from the foci, we still recover bbb.

The minor radius is determined from the combination of the surface point and the two foci.

The only thing I would criticize in the original definition is the wording:

"...defined by focus_1, focus_2 and geometry/centre (point on the surface) coordinates"

because "centre" and "point on the surface" are different concepts. If geometry/centre is actually intended to be a surface point, then the parameter name is misleading. But the geometric definition itself is sound. Probably removing the wording "(point on the surface)" would remove the confusion [and the IDS defines the geometry/center] but leaves the correct concept.