Laser DFB model #18

adophobr · 2023-12-13T17:47:50Z

No description provided.

edsonportosilva · 2023-12-14T11:55:42Z

Padronizar a documentação das funções. Tem função documentada de um jeito e função documentada de outro.
Adicionar os arquivos para a página de documentação (https://opticommpy.readthedocs.io/en/latest/index.html). Ver exemplo de módulo: (https://github.com/edsonportosilva/OptiCommPy/blob/main/optic/models/devices.py) e (https://github.com/edsonportosilva/OptiCommPy/blob/main/docs/source/models.rst)

Copilot

Pull request overview

This PR introduces a comprehensive semiconductor Distributed Feedback (DFB) laser simulation model to the optic package. The implementation provides rate equation solvers and intensity modulation response calculations for laser dynamics analysis.

Key Changes:

Added complete DFB laser model with rate equations solver
Implemented intensity modulation (IM) frequency response calculations
Added noise modeling capabilities for laser simulations

Reviewed changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated 12 comments.

File	Description
requirements.txt	Formatting fix: added missing line number to sphinx-rtd-theme entry
optic/models/laser.py	New module implementing DFB laser model with rate equations, noise sources, IM response calculations, and plotting utilities
docs/source/models.rst	Added documentation reference for the new laser module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T18:17:36Z

optic/models/laser.py

+        print('Invalid type of IM response.')
+        return -1


Using print() for error handling is not a best practice. Consider raising an exception (e.g., ValueError or TypeError) instead so that the caller can properly handle the error condition. The current implementation returns -1 which could lead to silent failures in downstream code.

Suggested change

print('Invalid type of IM response.')

return -1

raise ValueError(f"Invalid type of IM response: {type!r}. Expected 'exact' or 'approx.'.")

Copilot · 2025-12-23T18:17:36Z

optic/models/laser.py

+    float
+        The electrical current at the specified time.
+    """
+    return np.real(current.signal[np.argwhere(t >= current.t)[-1]])


The get_current function may fail with an IndexError if t is less than all values in current.t. The expression np.argwhere(t >= current.t)[-1] will raise an IndexError on an empty array. Consider adding boundary checking or handling this edge case.

Suggested change

return np.real(current.signal[np.argwhere(t >= current.t)[-1]])

idxs = np.argwhere(t >= current.t)

if idxs.size == 0:

# If t is earlier than all sampled times, use the earliest available current.

idx = 0

else:

idx = idxs[-1]

return np.real(current.signal[idx])

Copilot · 2025-12-23T18:17:36Z

optic/models/laser.py

+    current = get_current(current, t)
+    return current/(e*param.v)-y[0]/param.tau_n-param.vg*param.a0*y[1]*(y[0]-param.n_t)/(1+param.epsilon*y[1])


The variable shadowing in this function is problematic. The function parameter current is reassigned on line 335, which shadows the original parameter and makes the code harder to understand. Consider using a different variable name for the extracted current value, such as i_current or current_value.

Suggested change

current = get_current(current, t)

return current/(e*param.v)-y[0]/param.tau_n-param.vg*param.a0*y[1]*(y[0]-param.n_t)/(1+param.epsilon*y[1])

current_value = get_current(current, t)

return current_value/(e*param.v)-y[0]/param.tau_n-param.vg*param.a0*y[1]*(y[0]-param.n_t)/(1+param.epsilon*y[1])

Copilot · 2025-12-23T18:17:37Z

optic/models/laser.py

+        #rtol = 1e-4,
+        #atol = 1e-6,


The commented-out tolerance parameters (rtol and atol) should either be removed if they're not needed, or uncommented with appropriate values if they're intended to be used. Leaving them commented without explanation reduces code clarity and maintainability.

Suggested change

#rtol = 1e-4,

#atol = 1e-6,

Copilot · 2025-12-23T18:17:37Z

optic/models/laser.py

+import numpy as np
+import matplotlib.pyplot as plt
+from scipy.fft import fft
+from scipy.integrate import solve_ivp,odeint


Missing comma in import statement. The imports on line 17 should have a space after the comma separating solve_ivp and odeint for consistency with PEP 8 style guidelines.

Suggested change

from scipy.integrate import solve_ivp,odeint

from scipy.integrate import solve_ivp, odeint

Copilot · 2025-12-23T18:17:38Z

optic/models/laser.py

+    _extracted_from_plot_4(t, ax, 0, 0, 'Optical Power [mW]')
+    ax[1,0].plot(1e9*t, 1e-9*np.real(y.chirp))
+    _extracted_from_plot_4(t, ax, 1, 0, 'Chirp [GHz]')
+    ax[0,1].plot(1e9*t, np.real(y.N))
+    _extracted_from_plot_4(t, ax, 0, 1, r'Carrier density $N(t)$ [cm$^{-3}$]')
+    ax[1,1].plot(1e9*t, np.real(y.S))
+    _extracted_from_plot_4(t, ax, 1, 1, r'Photon Density $S(t)$ [cm$^{-3}$]')
+    plt.tight_layout()
+    return ax
+
+def _extracted_from_plot_4(t, ax, arg1, arg2, arg3):
+    ax[arg1, arg2].set_xlabel('Time [ns]')
+    ax[arg1, arg2].set_ylabel(arg3)
+    ax[arg1, arg2].set_xlim(1e9*np.array([t.min(),t.max()]))
+    ax[arg1, arg2].grid(True)


The function name _extracted_from_plot_4 is not descriptive and appears to be an auto-generated refactoring artifact. Consider using a more meaningful name like _set_subplot_properties or _configure_subplot_axes to better describe what this helper function does.

Suggested change

_extracted_from_plot_4(t, ax, 0, 0, 'Optical Power [mW]')

ax[1,0].plot(1e9*t, 1e-9*np.real(y.chirp))

_extracted_from_plot_4(t, ax, 1, 0, 'Chirp [GHz]')

ax[0,1].plot(1e9*t, np.real(y.N))

_extracted_from_plot_4(t, ax, 0, 1, r'Carrier density $N(t)$ [cm$^{-3}$]')

ax[1,1].plot(1e9*t, np.real(y.S))

_extracted_from_plot_4(t, ax, 1, 1, r'Photon Density $S(t)$ [cm$^{-3}$]')

plt.tight_layout()

return ax

def _extracted_from_plot_4(t, ax, arg1, arg2, arg3):

ax[arg1, arg2].set_xlabel('Time [ns]')

ax[arg1, arg2].set_ylabel(arg3)

ax[arg1, arg2].set_xlim(1e9*np.array([t.min(),t.max()]))

ax[arg1, arg2].grid(True)

_set_subplot_properties(t, ax, 0, 0, 'Optical Power [mW]')

ax[1,0].plot(1e9*t, 1e-9*np.real(y.chirp))

_set_subplot_properties(t, ax, 1, 0, 'Chirp [GHz]')

ax[0,1].plot(1e9*t, np.real(y.N))

_set_subplot_properties(t, ax, 0, 1, r'Carrier density $N(t)$ [cm$^{-3}$]')

ax[1,1].plot(1e9*t, np.real(y.S))

_set_subplot_properties(t, ax, 1, 1, r'Photon Density $S(t)$ [cm$^{-3}$]')

plt.tight_layout()

return ax

def _set_subplot_properties(t, ax, row, col, ylabel):

ax[row, col].set_xlabel('Time [ns]')

ax[row, col].set_ylabel(ylabel)

ax[row, col].set_xlim(1e9*np.array([t.min(),t.max()]))

ax[row, col].grid(True)

Copilot · 2025-12-23T18:17:38Z

optic/models/laser.py

+    out = parameters()
+    # get physical parameters
+    out.N = y[0,:]
+    out.S = y[1,: ]


There's an extra space in the array indexing y[1,: ]. For consistency with the rest of the code (see line 177), this should be y[1,:] without the space before the closing bracket.

Suggested change

out.S = y[1,: ]

out.S = y[1,:]

Copilot · 2025-12-23T18:17:38Z

optic/models/laser.py

+    """
+    # diffusion coefficient - see ref. [2]
+    dss = (param.beta*y[0]*y[1]/param.tau_n)
+    #dnn = (y[0]/self.tau_n)*(1+self.beta*y[1])


The commented-out line #dnn = (y[0]/self.tau_n)*(1+self.beta*y[1]) references self which doesn't exist in this function context. This appears to be leftover code from a class-based implementation. Either remove this comment if it's not needed, or update it to reflect the current function-based implementation if it's meant to be informative.

Suggested change

#dnn = (y[0]/self.tau_n)*(1+self.beta*y[1])

# dnn = (y[0] / param.tau_n) * (1 + param.beta * y[1])

Copilot · 2025-12-23T18:17:39Z

optic/models/laser.py

+"""Basic physical models for DFB laser."""
+import numpy as np
+import matplotlib.pyplot as plt
+from scipy.fft import fft


The import fft from scipy.fft is unused. Consider removing unused imports to keep the code clean and reduce dependencies.

Suggested change

from scipy.fft import fft

Copilot · 2025-12-23T18:17:39Z

optic/models/laser.py

+    dy = np.zeros(3)        
+    # Carrier density
+    dy[0] = carrier_density(t,y,param,current)
+    # Foton density


The comment "Foton density" contains a spelling error. The correct spelling is "Photon density".

Suggested change

# Foton density

# Photon density

edsonportosilva · 2025-12-23T18:28:35Z

@copilot open a new pull request to apply changes based on the comments in this thread

Additionally, refactor the functions names and documentation to make them compatible with the standard followed in the remaining of the repository.

Copilot

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T19:50:21Z

optic/models/laser.py

+    """
+    # diffusion coefficient - see ref. [2]
+    dss = (param.beta*y[0]*y[1]/param.tau_n)
+    #dnn = (y[0]/self.tau_n)*(1+self.beta*y[1])


Commented-out code found in commented line referencing 'self.tau_n' and 'self.beta'. This appears to be leftover code from a class-based implementation. Consider removing commented-out code or adding a comment explaining why it's preserved.

Suggested change

#dnn = (y[0]/self.tau_n)*(1+self.beta*y[1])

Copilot · 2025-12-23T19:50:21Z

optic/models/laser.py

+def _extracted_from_plot_4(t, ax, arg1, arg2, arg3):
+    ax[arg1, arg2].set_xlabel('Time [ns]')
+    ax[arg1, arg2].set_ylabel(arg3)
+    ax[arg1, arg2].set_xlim(1e9*np.array([t.min(),t.max()]))
+    ax[arg1, arg2].grid(True)


The function name '_extracted_from_plot_4' appears to be auto-generated by a refactoring tool. Consider renaming this to a more descriptive name like '_set_plot_properties' or '_configure_subplot_axis' to improve code readability.

Copilot · 2025-12-23T19:50:21Z

optic/models/laser.py

+           Mode confinement factor (default: 0.4).
+        - tau_p : float, optional
+            Photon lifetime (default: 3.0e-12).
+        - beta : float, optional
+           Fraction of spontaneous emission coupling (default: 3.0e-5).


Inconsistent indentation in docstring parameter descriptions. Line 44 has only one space for indentation while line 43 correctly has three spaces. The same issue appears on line 48.

Suggested change

Mode confinement factor (default: 0.4).

- tau_p : float, optional

Photon lifetime (default: 3.0e-12).

- beta : float, optional

Fraction of spontaneous emission coupling (default: 3.0e-5).

Mode confinement factor (default: 0.4).

- tau_p : float, optional

Photon lifetime (default: 3.0e-12).

- beta : float, optional

Fraction of spontaneous emission coupling (default: 3.0e-5).

Copilot · 2025-12-23T19:50:22Z

optic/models/laser.py

+    dy = np.zeros(3)        
+    # Carrier density
+    dy[0] = carrier_density(t,y,param,current)
+    # Foton density


There is a spelling error in the comment. "Foton" should be "Photon".

Suggested change

# Foton density

# Photon density

Copilot · 2025-12-23T19:50:22Z

optic/models/laser.py

+    param.tau_p = getattr(param, "tau_p", 3.0e-12)
+    param.beta = getattr(param, "beta", 3.0e-5)
+    param.alpha = getattr(param, "alpha", 5)
+    param.sigma = getattr(param, "sigma", 2e-20)


The parameter 'sigma' is set but never used anywhere in the code. Consider removing this unused parameter or documenting why it's reserved for future use.

Copilot · 2025-12-23T19:50:22Z

optic/models/laser.py

+    param.i_bias = getattr(param, 'i_bias', 0.078)
+    param.i_max  = getattr(param, 'i_max', 0.188)
+    param.eta_0 = getattr(param, 'eta_0', 0.4)
+    param.lmbd = getattr(param, 'lmbd', 1300e-9)


Inconsistent quote style for string keys. Line 96 uses single quotes for 'i_bias', 'i_max', and 'eta_0' (lines 96-98), while line 99 uses 'lmbd'. This is inconsistent with the double quotes used in earlier lines (e.g., lines 84-95). Consider standardizing on either single or double quotes throughout the function.

Suggested change

param.i_bias = getattr(param, 'i_bias', 0.078)

param.i_max = getattr(param, 'i_max', 0.188)

param.eta_0 = getattr(param, 'eta_0', 0.4)

param.lmbd = getattr(param, 'lmbd', 1300e-9)

param.i_bias = getattr(param, "i_bias", 0.078)

param.i_max = getattr(param, "i_max", 0.188)

param.eta_0 = getattr(param, "eta_0", 0.4)

param.lmbd = getattr(param, "lmbd", 1300e-9)

adophobr added 9 commits February 21, 2024 08:27

Create laser.py

de0509f

Update laser module with example.

96b9afa

Update test_laser

4a9bfd0

Update test_laser_cartledge_paper.ipynb

b1695ef

Include two examples into respective folder.

0aa3351

Update laser doc.

29359b5

Add new doctrings an fatatin example.

09bc8c9

Update laser.py

4f12507

Move teste_laser_fatatin_paper.

659e29e

adophobr force-pushed the laser branch from a829586 to 659e29e Compare February 21, 2024 11:28

adophobr added 2 commits February 21, 2024 09:44

Update laser test files and requirements.

9aca66d

Update requirements.txt

a25856c

edsonportosilva requested a review from Copilot December 23, 2025 18:14

Copilot started reviewing on behalf of edsonportosilva December 23, 2025 18:14 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

edsonportosilva requested a review from Copilot December 23, 2025 19:45

Copilot started reviewing on behalf of edsonportosilva December 23, 2025 19:46 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

	print('Invalid type of IM response.')
	return -1
	raise ValueError(f"Invalid type of IM response: {type!r}. Expected 'exact' or 'approx.'.")

-    return np.real(current.signal[np.argwhere(t >= current.t)[-1]])
+    idxs = np.argwhere(t >= current.t)
+    if idxs.size == 0:
+        # If t is earlier than all sampled times, use the earliest available current.
+        idx = 0
+    else:
+        idx = idxs[-1]
+    return np.real(current.signal[idx])

		current = get_current(current, t)
		return current/(eparam.v)-y[0]/param.tau_n-param.vgparam.a0y[1](y[0]-param.n_t)/(1+param.epsilon*y[1])

	from scipy.integrate import solve_ivp,odeint
	from scipy.integrate import solve_ivp, odeint

	#dnn = (y[0]/self.tau_n)(1+self.betay[1])
	# dnn = (y[0] / param.tau_n) * (1 + param.beta * y[1])

Laser DFB model #18

Are you sure you want to change the base?

Laser DFB model #18

Uh oh!

Conversation

adophobr commented Dec 13, 2023

Uh oh!

edsonportosilva commented Dec 14, 2023

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

edsonportosilva commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants