Simplify Maxwell external source

Description

Simplify Maxwell external source and merge it with plane waves.

News snippet

Simplify Maxwell external source and merge it with plane waves.

Checklist

• I have checked that my code follows the Octopus coding standards
• I have added tests for all the new features added in this request.

changed milestone to %14.0

added Multisystem Refactoring labels

assigned to @micael.oliveira

approved this merge request

@micael.oliveira Nice changes ! I am approving in advance.

@ilke_3 @fbonafe @heiko.appel I think it would be good if you could have a look at these changes. The first commit (b8d85218) shows how this should have been implemented in the first place, but besides that, I have a few questions. In particular, I find the names used here very confusing: there's an external source, but the only type of source it knows is a plane wave object. Then at a quick glance, it seems that object does not deal only with plane waves. Could you clarify this a bit? I'm asking because after the changes introduced in the first commit, it is trivial to make plane_wave_t an interaction partner, such that external_source_t becomes unnecessary. This is implemented in the second commit (56a9b025), but then the names used in the code and the input options all become inconsistent.

@AlexBuccheri I think you were the reviewer of the original merge request that implemented the Maxwell external sources, so I think it would be good if you could also review this (regardless of @nicolastd enthusiastic approval ).

Thanks a lot @micael.oliveira, this is a clever way of doing it. With Ilke originally we thought defining plane waves as the partner would not work, as Maxwell also needs to call plane waves init and eval when we're setting the pw boundaries, but clearly it does work.

Regarding the naming of the plane_waves_t, indeed this started to be incorrect when the Bessel beams were added. Could you please rename the module and the type to "external_waves_oct_t" and "external_waves_t"? The input variable is already called MaxwellIncidentWaves so there is no problem leaving it as it is.

added 1 commit

• c20acddd - Rename plane_wave_t to external_waves_t.

Compare with previous version

marked this merge request as ready

Could you please rename the module and the type to "external_waves_oct_t" and "external_waves_t"?

@fbonafe Done!

approved this merge request

approved this merge request

merged

mentioned in commit efb84b14

requested review from @fbonafe

requested review from @nicolastd, @heiko.appel, and @ilke_3

removed review request for @heiko.appel

added in Changelog label