Skip to content

z-dependent core radius#99

Merged
chrisbrahms merged 35 commits intoLupoLab:masterfrom
chrisbrahms:taper
Apr 1, 2020
Merged

z-dependent core radius#99
chrisbrahms merged 35 commits intoLupoLab:masterfrom
chrisbrahms:taper

Conversation

@chrisbrahms
Copy link
Copy Markdown
Collaborator

Implements a slightly hacky version of varying the core radius in capillary modes. For the dispersion this works correctly (I think) and uses the same mechanism as I implemented for gradients. For the nonlinearity, at the moment I'm multiplying by a factor in TransModeAvg to take into account the changing effective area. This means that the output field will also have to be rescaled after the propagation (I haven't implemented this).

I have nothing to verify this with, but constant core radius reproduces the results in test_main.jl and a reducing radius does something plausible (RDW at shorter wavelength, shorter fission length)

@chrisbrahms chrisbrahms requested a review from jtravs February 6, 2020 09:29
@jtravs
Copy link
Copy Markdown
Contributor

jtravs commented Feb 6, 2020

I believe this will almost already work for modal fields too. I think the only addition required is to make dimlimits a function.

For the mode average, I think we should switch to propagating the flux rather than the field (as we do for modal problems). This will also remove some code as the norm_ and energy_ functions will become unified.

But as a hack this looks OK.

@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

Now much less hacky with the switch to propagating the flux also for the mode average. This is implemented by still applying a scaling factor to the field before calculating the nonlinearity and changing the norm to include the inverse of this scaling factor.

@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

Now with analytical calculations for Aeff and N - execution time down from ~100-200 μs to <100 ns.

Question: why do we limit TE/TM modes to m=1? As far as I can see, higher-order modes just have different unm and nothing else changes.

@jtravs
Copy link
Copy Markdown
Contributor

jtravs commented Mar 26, 2020

What's the status of this? Is it no longer "hacky".

@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

I think there were a couple of small issues and the tests aren't quite comprehensive enough yet. Also all the examples need to be changed. And I guess we can unify energy_modal and energy_mode_avg now, given that they're the same

Comment thread src/NonlinearRHS.jl
end
return energyfun
end
_energy_modal(t, Et::Array{T, N}) where T <: Real where N = _energy_modal(t, Maths.hilbert(Et))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure about this. It makes everything very neat but relies on envelope fields actually being complex--which they are not necessarily, at least at the beginning of the propagation. Hence the fixtype function in Luna. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it is OK, but see my comment below

Comment thread src/Luna.jl Outdated
@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

We now have analytical N and Aeff for capillary and rectangular modes. I've fixed all the tests that use mode averaging to include the new normalisation, and tests pass now (except the broken ones). I have not fixed all the examples.

Another quick review would be good, but then I think this is actually ready to go

@chrisbrahms chrisbrahms changed the title (do not merge) simple tapering for mode average z-dependent core radius Mar 31, 2020
@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

One thing I haven't done is actually implement the z-dependent dimensions for RectModes. Will this be needed any time soon?

@jtravs
Copy link
Copy Markdown
Contributor

jtravs commented Mar 31, 2020

Not immediately, but it would be useful. I'd be happy for this to be merged and an issue opened to track that. What actually needs to be done here?

@chrisbrahms chrisbrahms mentioned this pull request Mar 31, 2020
4 tasks
jtravs added a commit that referenced this pull request Mar 31, 2020
OK, I'll leave that to be fixed in #99
Comment thread test/test_main_rect.jl
aeff(z) = Modes.Aeff(m, z=z)

energyfun = NonlinearRHS.energy_mode_avg(m)
energyfun = NonlinearRHS.energy_modal()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we not just call this function energy given that it is now universal?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it's not universal though, given that #85 is nearly ready too. I have to double check but we can probably call it energy anyway (this is a good idea) and just return the correct function depending on the arguments. I will probably change the energy calculation a bit anyway, for fast statistics and possibly for more universal inputs as well

Copy link
Copy Markdown
Contributor

@jtravs jtravs left a comment

Choose a reason for hiding this comment

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

I've read through it again and this looks good. Moved on considerably from the "hacky" first attempt!

@chrisbrahms chrisbrahms merged commit 2eb2030 into LupoLab:master Apr 1, 2020
@jtravs
Copy link
Copy Markdown
Contributor

jtravs commented Apr 1, 2020

The gradient tests still fail for me even after merging this.

@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

I've tracked it down to the change in Maths.derivative in this commit but I have no idea why yet

@chrisbrahms
Copy link
Copy Markdown
Collaborator Author

Test passes if, instead of doing this,
https://github.com/LupoLab/Luna/blob/2eb20302aa6ce8d356b14d6aade9d2c3c6dcb3b2/src/LinearOps.jl#L19
I just pass β1 into the function instead and use that directly:

function make_const_linop(grid::Grid.RealGrid, βfun!, αfun!, β1)
    β = similar(grid.ω)
    βfun!(β, grid.ω, 0)
    α = similar(grid.ω)
    αfun!(α, grid.ω, 0)
    αlim!(α)
    linop = @. -im*-β1*grid.ω) - α/2
    linop[1] = 0
    return linop
end

Some weird floating-point error caused by dividing and dividing again, maybe.

@chrisbrahms chrisbrahms mentioned this pull request Apr 1, 2020
@jtravs
Copy link
Copy Markdown
Contributor

jtravs commented Apr 1, 2020

OK.
We need to systematically review the derivative business again properly. I'll track what happens upstream on Finite Differences, then we should check the best way to handle this.

@jtravs jtravs mentioned this pull request Apr 1, 2020
@chrisbrahms chrisbrahms deleted the taper branch April 17, 2020 14:05
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