Skip to content

Conversation

damiano-flex
Copy link
Contributor

@damiano-flex damiano-flex commented Sep 10, 2025

Add SSAC Voltage source class.

Greptile Summary

Updated On: 2025-09-10 09:22:23 UTC

This PR adds support for Small Signal AC (SSAC) voltage sources to the Tidy3D simulation framework. The implementation introduces a new SSACVoltageSource class that enables AC analysis in heat-charge simulations by providing DC bias voltages, AC frequencies, and small-signal amplitudes.

The core change is the addition of the SSACVoltageSource class in a new module (tidy3d/components/spice/sources/ac.py), which follows the established patterns from the existing DCVoltageSource class. The class includes three main parameters: voltage (array of DC bias voltages), frequency (array of AC analysis frequencies), and amplitude (small-signal AC magnitude). All fields include proper validation to ensure finite values and positive frequencies.

The implementation integrates seamlessly with the existing SPICE sources framework by updating the VoltageSourceType union to include the new AC source alongside the existing DC source. A validation constraint has been added to the HeatChargeSimulation class to enforce that only a single AC source can be present in a simulation's boundary conditions, which prevents ambiguous analysis scenarios.

The new functionality is exposed through the main package interface by adding the class to the imports and __all__ list in tidy3d/__init__.py. This follows the established pattern where SPICE-like analysis components are made available as part of the public API.

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/components/spice/sources/ac.py 4/5 New file implementing SSACVoltageSource class for small-signal AC analysis with proper validation
tidy3d/components/tcad/simulation/heat_charge.py 4/5 Added validator to enforce single AC source constraint in simulations
tests/test_components/test_heat_charge.py 5/5 Added comprehensive validation tests for SSACVoltageSource and multiple AC source constraints
tidy3d/components/spice/sources/types.py 5/5 Updated VoltageSourceType union to include new SSACVoltageSource
tidy3d/__init__.py 5/5 Added SSACVoltageSource to main package imports and exports

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it adds new functionality without modifying existing behavior
  • Score reflects well-structured implementation following established patterns, though the single AC source constraint may need documentation
  • Pay close attention to tidy3d/components/tcad/simulation/heat_charge.py to ensure the validation logic is complete

Sequence Diagram

sequenceDiagram
    participant User
    participant HeatChargeSimulation
    participant VoltageBC
    participant SSACVoltageSource
    participant ValidationEngine

    User->>HeatChargeSimulation: "Create simulation with VoltageBC"
    HeatChargeSimulation->>VoltageBC: "Initialize VoltageBC"
    VoltageBC->>SSACVoltageSource: "Initialize SSACVoltageSource"
    SSACVoltageSource->>ValidationEngine: "Validate voltage array"
    ValidationEngine-->>SSACVoltageSource: "Check for infinite values"
    ValidationEngine-->>SSACVoltageSource: "Validation result"
    SSACVoltageSource-->>VoltageBC: "Source initialized"
    VoltageBC-->>HeatChargeSimulation: "Boundary condition created"
    HeatChargeSimulation->>ValidationEngine: "Validate single SSAC source"
    ValidationEngine-->>HeatChargeSimulation: "Check boundary specs for multiple AC sources"
    ValidationEngine-->>HeatChargeSimulation: "Validation result"
    HeatChargeSimulation-->>User: "Simulation created with SSAC voltage source"
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Reviewing changes made in this pull request

@damiano-flex damiano-flex changed the title Add SSAC voltage source feat: Add SSAC voltage source Sep 10, 2025
Notes
-----
- The bias `voltage` refer to the DC operating point above the simulation ground.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be RST syntax here

 ``voltage``

As a rule, we don't use markdown notation in the docs. It's either RST (two "`"-s) in docstrings, or single quotes ' in warning/error messages that will be printed to stdout.

voltage: ArrayFloat1D = pd.Field(
...,
title="DC Bias Voltages",
description="List of DC operating point voltages (above ground) used with 'VoltageBC'.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also double ` here as it gets converted to a docstring.

-----
- The bias `voltage` refer to the DC operating point above the simulation ground.
- The `frequency` array lists the AC analysis frequencies (in Hz).
- The `amplitude` is the small-signal AC magnitude (in Volts).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the "frequency" and "amplitude" Note are not really needed given that the information is also given in the field description.

frequency: ArrayFloat1D = pd.Field(
...,
title="AC Analysis Frequencies",
description="List of small-signal analysis frequencies (in Hz).",
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to say the units in the description if they are defined in the units argument below, as they should be. See for example how this is parsed for the existing VoltageBC:

image

ac_current_voltage: Optional[FreqPotentialDataArray] = pd.Field(
None,
title="AC current vs voltage and frequency",
description="Complex small-signal current I(v, f) as a FreqPotentialDataArray.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, no need to say as a FreqPotentialDataArray here. Since the field type is Optional[FreqPotentialDataArray], this information will already appear in the parsed docstring, including a clickable link to the type.

bc.condition.source.amplitude,
)
raise SetupError(
"'HeatSimulation' does not include any `SSACVoltageSource` in 'boundary_spec')."
Copy link
Collaborator

Choose a reason for hiding this comment

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

' everywhere in error messages.

@pd.validator("amplitude")
def validate_amplitude(cls, val):
if val == td_inf:
raise ValueError(f"Small signal amplitude must be finite. Current amplitude={val}.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking out loud here, we want the amplitude to not just be finite, but "small" in some sense. Is there something that we can test it against and at least issue a warning if it doesn't seem to be the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

For this type of analysis the perturbed output varies linearly (i.e., perturbation n, p, phi) with the amplitude so in principle in doesn't matter the amplitude. A different story is whether the solution is physically meaningful. I think it is, in general, not straightforward to define a validity limit and it may indeed be the case that it is case-dependent.

It won't hurt, however, to include some warning or note in the docstring mentioning the validity of the simulation.

Copy link
Contributor

@marc-flex marc-flex left a comment

Choose a reason for hiding this comment

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

Thanks @damiano-flex

I left a few comments but this is looking good!

I guess a more general comment, Do we want to include field output? Such as maybe perturbation current, perturbation potential etc?

@pd.validator("amplitude")
def validate_amplitude(cls, val):
if val == td_inf:
raise ValueError(f"Small signal amplitude must be finite. Current amplitude={val}.")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this type of analysis the perturbed output varies linearly (i.e., perturbation n, p, phi) with the amplitude so in principle in doesn't matter the amplitude. A different story is whether the solution is physically meaningful. I think it is, in general, not straightforward to define a validity limit and it may indeed be the case that it is case-dependent.

It won't hurt, however, to include some warning or note in the docstring mentioning the validity of the simulation.

Comment on lines 535 to 547
class FreqPotentialDataArray(DataArray):
"""Frequency-domain array.
Example
-------
>>> f = [2e14, 3e14]
>>> v = [0.1, 0.2, 0.3]
>>> coords = dict(f=f, v=v)
>>> fd = FreqPotentialDataArray((1+1j) * np.random.random((2, 3)), coords=coords)
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want FreqPotentialDataArray or FreqVoltageDataArray? I think it'd be more consistent with the other classes with voltage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I vaguely remember this was already introduced for RF hmm

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fyi

class VoltageFreqDataArray(VoltageArray, FreqDataArray):
"""Voltage data array in frequency domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9, 4e9]
>>> coords = dict(f=f)
>>> data = np.random.random(3) + 1j * np.random.random(3)
>>> vfd = VoltageFreqDataArray(data, coords=coords)
"""
__slots__ = ()

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was all recently added fyi

class VoltageArray(DataArray):
# Always set __slots__ = () to avoid xarray warnings
__slots__ = ()
_data_attrs = {"units": VOLT, "long_name": "voltage"}
class CurrentArray(DataArray):
# Always set __slots__ = () to avoid xarray warnings
__slots__ = ()
_data_attrs = {"units": AMP, "long_name": "current"}
class ImpedanceArray(DataArray):
# Always set __slots__ = () to avoid xarray warnings
__slots__ = ()
_data_attrs = {"units": OHM, "long_name": "impedance"}
# Voltage arrays
class VoltageFreqDataArray(VoltageArray, FreqDataArray):
"""Voltage data array in frequency domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9, 4e9]
>>> coords = dict(f=f)
>>> data = np.random.random(3) + 1j * np.random.random(3)
>>> vfd = VoltageFreqDataArray(data, coords=coords)
"""
__slots__ = ()
class VoltageTimeDataArray(VoltageArray, TimeDataArray):
"""Voltage data array in time domain.
Example
-------
>>> import numpy as np
>>> t = [0, 1e-9, 2e-9, 3e-9]
>>> coords = dict(t=t)
>>> data = np.sin(2 * np.pi * 1e9 * np.array(t))
>>> vtd = VoltageTimeDataArray(data, coords=coords)
"""
__slots__ = ()
class VoltageFreqModeDataArray(VoltageArray, FreqModeDataArray):
"""Voltage data array in frequency-mode domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9]
>>> mode_index = [0, 1]
>>> coords = dict(f=f, mode_index=mode_index)
>>> data = np.random.random((2, 2)) + 1j * np.random.random((2, 2))
>>> vfmd = VoltageFreqModeDataArray(data, coords=coords)
"""
__slots__ = ()
# Current arrays
class CurrentFreqDataArray(CurrentArray, FreqDataArray):
"""Current data array in frequency domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9, 4e9]
>>> coords = dict(f=f)
>>> data = np.random.random(3) + 1j * np.random.random(3)
>>> cfd = CurrentFreqDataArray(data, coords=coords)
"""
__slots__ = ()
class CurrentTimeDataArray(CurrentArray, TimeDataArray):
"""Current data array in time domain.
Example
-------
>>> import numpy as np
>>> t = [0, 1e-9, 2e-9, 3e-9]
>>> coords = dict(t=t)
>>> data = np.cos(2 * np.pi * 1e9 * np.array(t))
>>> ctd = CurrentTimeDataArray(data, coords=coords)
"""
__slots__ = ()
class CurrentFreqModeDataArray(CurrentArray, FreqModeDataArray):
"""Current data array in frequency-mode domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9]
>>> mode_index = [0, 1]
>>> coords = dict(f=f, mode_index=mode_index)
>>> data = np.random.random((2, 2)) + 1j * np.random.random((2, 2))
>>> cfmd = CurrentFreqModeDataArray(data, coords=coords)
"""
__slots__ = ()
# Impedance arrays
class ImpedanceFreqDataArray(ImpedanceArray, FreqDataArray):
"""Impedance data array in frequency domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9, 4e9]
>>> coords = dict(f=f)
>>> data = 50.0 + 1j * np.random.random(3)
>>> zfd = ImpedanceFreqDataArray(data, coords=coords)
"""
__slots__ = ()
class ImpedanceTimeDataArray(ImpedanceArray, TimeDataArray):
"""Impedance data array in time domain.
Example
-------
>>> import numpy as np
>>> t = [0, 1e-9, 2e-9, 3e-9]
>>> coords = dict(t=t)
>>> data = 50.0 * np.ones_like(t)
>>> ztd = ImpedanceTimeDataArray(data, coords=coords)
"""
__slots__ = ()
class ImpedanceFreqModeDataArray(ImpedanceArray, FreqModeDataArray):
"""Impedance data array in frequency-mode domain.
Example
-------
>>> import numpy as np
>>> f = [2e9, 3e9]
>>> mode_index = [0, 1]
>>> coords = dict(f=f, mode_index=mode_index)
>>> data = 50.0 + 10.0 * np.random.random((2, 2))
>>> zfmd = ImpedanceFreqModeDataArray(data, coords=coords)
"""
__slots__ = ()

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @daquinteroflex!
Last time I spoke to @dmarek-flex we kind of reserved "voltage" to whenever we're comparing potentials and "potential" when we talk about the field. But maybe @dmarek-flex can confirm

Copy link
Contributor

@dmarek-flex dmarek-flex Sep 19, 2025

Choose a reason for hiding this comment

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

yea, same units but conceptually fairly different. Not a strong feeling, but I do think that FreqPotentialDataArray is a better description that also differentiates it more from the VoltageArrays we introduced in RF.

Sadly if we don't modify we will have FreqVoltageDataArray and VoltageFreqDataArray 😞

Copy link
Contributor

@dmarek-flex dmarek-flex Sep 19, 2025

Choose a reason for hiding this comment

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

We still don't have an enforced naming scheme for DataArray, but one idea is to have the leading term prefix relate to the quantity (Voltage/Current/Potential) and the remaining terms relate to the coordinates (Freq/Mode/Spatial). So maybe follow that approach

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I might be confused about the purpose of FreqVoltageDataArray. The coordinates are frequency and voltage? In that case, I agree with the current approach 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, coordinates are frequency and voltage.

This source represents a small-signal AC excitation defined by a list of bias voltages,
a set of small-signal analysis frequencies, and a small-signal amplitude (linear scale).
The bias ``voltage`` refer to the DC operating point above the simulation ground. Currently, electrical ports are not defined.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • refer -> refers
  • We don't really define a ground, do we? We define a voltage difference. Or have we changed this for SSAC?

Copy link
Contributor Author

@damiano-flex damiano-flex Sep 18, 2025

Choose a reason for hiding this comment

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

Yes, I had a discussion with @daquinteroflex about this. We need to discuss about on how and where let the user set the ground.

bc.condition.source.amplitude,
)
raise SetupError(
"'HeatSimulation' does not include any 'SSACVoltageSource' in 'boundary_spec')."
Copy link
Contributor

Choose a reason for hiding this comment

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

HeatSimulation is deprecated. We should use HeatChargeSimulation instead

Comment on lines 1935 to 1951
def _get_voltage_regime(self):
for bc in self.boundary_spec:
if isinstance(bc.condition, VoltageBC):
if isinstance(bc.condition.source, SSACVoltageSource):
return "ac"
return "dc"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could generalize this to something like _get_charge_type().
Right now we only need to modify the name while we keep the body of the function but I can see how this can be also helpful as we move into transient (time-domain) simulations.

@marc-flex marc-flex force-pushed the marc/non_isothermal branch 3 times, most recently from aae214d to fe57931 Compare September 18, 2025 14:01
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good re what we discussed
Screenshot from 2025-09-18 11-44-15

I guess for this type of analysis we're not specifying the type of AC signal shape?
Screenshot from 2025-09-18 11-47-26

and so you're not envisioning adding a type? Or should we introduce signal shape types even if say this defaults to sine?

Comment on lines 46 to 63
frequency: ArrayFloat1D = pd.Field(
...,
title="AC Analysis Frequencies",
description="List of small-signal analysis frequencies.",
units=HERTZ,
)

amplitude: pd.FiniteFloat = pd.Field(
...,
title="Small-Signal Amplitude",
description="Small-signal AC amplitude.",
units=VOLT,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So yeah wondering if we should specify this via something like

class SinusodialSignalType
    frequency: ArrayFloat1D = pd.Field(
        ...,
        title="AC Analysis Frequencies",
        description="List of small-signal analysis frequencies.",
        units=HERTZ,
    )

    amplitude: pd.FiniteFloat = pd.Field(
        ...,
        title="Small-Signal Amplitude",
        description="Small-signal AC amplitude.",
        units=VOLT,
    )

and then:

class ACVoltageSource(Tidy3dBaseModel):

... 

 signal_parameters: SignalType

Just hacked this quickly but maybe also need to think about names

Apart from this rest of the PR looks good

Copy link
Collaborator

Choose a reason for hiding this comment

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

Feel this would be pretty useful for the future too, also suspect maybe we already anyways have somthing like this for some time domain sources used elsewhere in the API

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that makes some sense @daquinteroflex, similarly to how we have our FDTD sources take a source_time which just defines the time dependence of the source, while the other parameters are related to the spatial extent, specific details of the modes, etc. On the other hand, doesn't amplitude make more sense to be an ACVoltageSource parameter than a SinusoidalSignal (because it's nice if the signal itself is dimensionless). But then that just leaves frequency in the SinusoidalSignal, so it makes it a bit pointless?

I don't think there's anything from the existing API we can reuse directly. One other thing I'm thinking about is that while this is technically a source, it also more closely matches the FDTD monitor-s. You can define a list of frequencies, and you will get output data that has a dimension over that list. This is exactly like our MonitorData. One point here is that we've been already inconsistent about the frequency labeling: we have Monitor.freqs, but the MonitorData has a corresponding f dimension. Shall we at least call frequency -> freqs here to be in line with our FDTD monitors?

Copy link
Collaborator

@daquinteroflex daquinteroflex Sep 19, 2025

Choose a reason for hiding this comment

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

So am thinking eventually we probably want to implement all these different ways of specifying time domain SPICE signal sources (either for voltage or current).

But feel maybe we need to be more specific whether we want to use an ACVoltageSource or simply all the specific signal shapes we want to support directly. I think @damiano-flex made it this way because of the way voltage sources are specified in SPICE

VXXXXXXX N+ N- <<DC> DC/TRAN VALUE> <AC <ACMAG <ACPHASE>>>
+ <DISTOF1 <F1MAG <F1PHASE>>> <DISTOF2 <F2MAG <F2PHASE>>>

translates to

name positive_node negative_node
[this specifies a DC bias <<DC> DC/TRAN VALUE>]
[this specifies a custom AC source <AC <ACMAG <ACPHASE>>> + <DISTOF1 <F1MAG <F1PHASE>>> <DISTOF2 <F2MAG <F2PHASE>>>]
image

So in this sense feeling this is why a ACSignalType should be defined as a standalone class. See 4.1 in https://ngspice.sourceforge.io/docs/ngspice-44-manual.pdf These are some examples of how they can be defined:

Based on these types of signal definitions, in my opinion I actually do think ampltude does make sense to live say in SinusosidalSignal (not my favorite name yet just to be consistent), because say a PulseSignal can be specifed between two voltages rather than just a single magnitude for example, and other AC sources require different parameters independent of amplitude too yet are still "AC" source compatible.

Sounds good, was not sure either what exactly was reusable. I do agree for consistency we can use freqs.

Screenshot from 2025-09-19 11-08-33 Screenshot from 2025-09-19 11-08-23 Screenshot from 2025-09-19 11-08-09

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK to use freqs for consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh yes, sounds good.

Those are definitely related to what we're doing here, but I was saying that I don't think our existing API can be used, because there's still some assumptions there that this is geared towards EM simulations. For example, the ContinuousWave class is basically a Sinusoidal, but, first of all, it's complex-valued, and second of all, it includes a linear ramp-up based on the fwidth parameter. For something as simple as a base amplitude and an offset amplitude where the source is something like V0 + V1*Sinusoidal, it still makes sense to me to follow this approach of defining a time-only class. I
That said, on second thought, if we do decide to separate time-only (unitless) definitions, this file could be a good place to put some new classes.

This seems a good approach in terms of API consistency, basically these abstract time classes would be an equivalent representation, and we will be able to translate to SPICE types if we need eventually too. Also worth getting @nikoladjor-flex input here. So creating those abstract time-shared classes seems good, and they can be used within ACVoltageSource and or eventually ACCurrentSource too in this approach. I do think it makes sense to define signal classes separate from the kind of physical quantity like Voltage that they modulate for reusability. Not sure what you think about that @damiano-flex ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daquinteroflex as we discussed in Slack, I don't have a strong opinion about what's the right thing to do. Here's what i think though.

Having a class SinusoidalSignal is not something inherently wrong to me, but providing a vector of frequencies to this class is. Classes in time.py are made to generate a single time-signal each, either by specifying some parameters (ContinousWave) or by specifying the value of the signal in time domain. Providing a vector of frequencies here still seems a bit off to me.

Copy link
Collaborator

@daquinteroflex daquinteroflex Sep 19, 2025

Choose a reason for hiding this comment

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

I agree, a vector would be weird in this case since it doesn't make sense physical sense unless it is a collection of signals. Feel this is a byproduct of wanting to integrate a sweep at the same time as a source potentially in the initial implementation, maybe instead of separating it into the analysis. Personally of the opinion parameter sweeps analysis to be separate from the source definition anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point @daquinteroflex. Maybe we could allow to have an array of ac sources?

Choose a reason for hiding this comment

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

Hey, the things I can add at the moment are more related to the application and I might be completely off-topic and wrong:

  • small signal analysis is usually done to determine frequency behaviour when you do not know the impedance of the device and want to get the bandwidth. Two most obvious applications for PIC are p/n modulators and photodetectors. Having array of sources here might help if there is a step in the simulation with brings the system to a steady state and then you apply small signals separately.
  • from the API point of view, this analysis makes sense on a single component, because you want three terminals the most. I tend to agree with @daquinteroflex about the abstract ACSource with added offset and frequency as list maybe. The point with the array of sources is the same as above. My main remark is that from the point of view of setting this analysis up, one needs to know which structures get what type of signal, and if we are already using BCs for that, it's fine, but we need to provide easy way to go from the circuit to the BC definition and from analysis result to impedance.

... )
"""

name: Optional[str]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of the name?

If it's needed, it should be formatted similarly to e.g. this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had no purposes for it in mind, I just used the same structure of DCVoltageSource (which has a field formatted like this one) . But in principle, having a name could be useful for future usage (e.g. a function that modify a source value through the name).

So, either I remove it in both, or I format it correctly here and in DCVoltageSource well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, let's format both, thanks.

@momchil-flex
Copy link
Collaborator

@damiano-flex @marc-flex I am getting confused with the source types and the analysis types. Would one still be defining e.g. IsothermalSteadyChargeDCAnalysis when there are AC type sources?

@damiano-flex
Copy link
Contributor Author

@damiano-flex @marc-flex I am getting confused with the source types and the analysis types. Would one still be defining e.g. IsothermalSteadyChargeDCAnalysis when there are AC type sources?

Initially, I had a different Analysis type for AC analysis. It was virtually a clone. Marc suggested to keep old analysis types, and define AC at source level, and this idea seemed good to me as well. If you have an AC source, you'll get everything from the DC simulation + a graph for AC current.

An option might be that IsothermalSteadyChargeDCAnalysis drops the DC keyword or change its name, since: 1) the DC analysis will be always performed 2) the AC analysis will be performed after the DC analysis if an AC source has been defined.

@momchil-flex
Copy link
Collaborator

@damiano-flex @marc-flex I am getting confused with the source types and the analysis types. Would one still be defining e.g. IsothermalSteadyChargeDCAnalysis when there are AC type sources?

Initially, I had a different Analysis type for AC analysis. It was virtually a clone. Marc suggested to keep old analysis types, and define AC at source level, and this idea seemed good to me as well. If you have an AC source, you'll get everything from the DC simulation + a graph for AC current.

An option might be that IsothermalSteadyChargeDCAnalysis drops the DC keyword or change its name, since: 1) the DC analysis will be always performed 2) the AC analysis will be performed after the DC analysis if an AC source has been defined.

I think that makes sense. Note that, for backwards compatibility, we'd keep IsothermalSteadyChargeDCAnalysis but issue a deprecation warning when using it. So something like

class IsothermalSteadyChargeAnalysis():
    # takes everything that's currently in IsothermalSteadyChargeDCAnalysis

class IsothermalSteadyChargeDCAnalysis(IsothermalSteadyChargeAnalysis):
    # add deprecation warning

Refer to the HeatSimulation.

@marc-flex marc-flex force-pushed the marc/non_isothermal branch 2 times, most recently from 7013f01 to 6c070c7 Compare September 19, 2025 12:28
@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch 2 times, most recently from e19ba56 to 02f85c4 Compare September 19, 2025 13:07
@marc-flex
Copy link
Contributor

@damiano-flex @momchil-flex I'm now having second thoughts about the current naming of sources and analysis types.

Right now we're saying that if we see an ACVoltageSource in a boundary condition we'll perform an SSAC simulation. But what if, in the future we can do non-linear simulations with AC sources? I guess we have two way forward:

  1. We create small-signal equivalents of the SteadyChargeAnalysis and IsothermalSteadyChargeAnalysis as @damiano-flex suggested in the first place; or
  2. We create SSACVoltageSource. I guess the only issue with this one is that it isn't very SPICE like

@daquinteroflex
Copy link
Collaborator

These type of charge simulations are different from FDTD simulations in the sense that a given "scene" with sources etc. can have different types of analysis depending on what you're looking into like transient or DC operating point, and so the data generated is inherently different per analysis type.

In any case:

We create small-signal equivalents of the SteadyChargeAnalysis and IsothermalSteadyChargeAnalysis as @damiano-flex suggested in the first place; or

would be then equivalent to this type of SS ac SPICE analysis directive that Damiano mentioned separately, but then the decision is whether the analysis type has the frequency sweep or the source. IMO it should be the analysis but use the source name to reference to it, and override any specified frequency in it.

.ac dec nd fstart fstop
.ac oct no fstart fstop
.ac lin np fstart fstop

Base automatically changed from marc/non_isothermal to develop September 19, 2025 13:43
Copy link
Contributor

github-actions bot commented Sep 19, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/init.py (100%)
  • tidy3d/components/data/data_array.py (100%)
  • tidy3d/components/frequencies.py (100%)
  • tidy3d/components/spice/analysis/ac.py (95.5%): Missing lines 49
  • tidy3d/components/spice/sources/ac.py (95.5%): Missing lines 67
  • tidy3d/components/spice/sources/dc.py (100%)
  • tidy3d/components/spice/sources/types.py (100%)
  • tidy3d/components/spice/types.py (100%)
  • tidy3d/components/tcad/data/sim_data.py (100%)
  • tidy3d/components/tcad/simulation/heat_charge.py (92.7%): Missing lines 2005-2006,2008,2012

Summary

  • Total: 122 lines
  • Missing: 6 lines
  • Coverage: 95%

tidy3d/components/spice/analysis/ac.py

Lines 45-53

  45 
  46     @pd.validator("ssac_freqs")
  47     def validate_ssac_freqs(cls, val):
  48         if len(val) == 0:
! 49             raise ValueError("'ssac_freqs' cannot be empty (size 0).")
  50         else:
  51             for freq in val:
  52                 if freq == td_inf:
  53                     raise ValueError("'ssac_freqs' cannot contain infinite frequencies.")

tidy3d/components/spice/sources/ac.py

Lines 63-69

  63 
  64     @pd.validator("amplitude")
  65     def validate_amplitude(cls, val):
  66         if val == td_inf:
! 67             raise ValueError(f"Signal amplitude must be finite. Current amplitude={val}.")
  68         return val

tidy3d/components/tcad/simulation/heat_charge.py

Lines 2001-2016

  2001 
  2002         return any(isinstance(source, HeatFromElectricSource) for source in self.sources)
  2003 
  2004     def _get_charge_type(self):
! 2005         if isinstance(self.analysis_spec, (SSACAnalysis, IsothermalSSACAnalysis)):
! 2006             return "ac"
  2007         else:
! 2008             return "dc"
  2009 
  2010     def _get_ssac_frequency_and_amplitude(self):
  2011         if not isinstance(self.analysis_spec, (SSACAnalysis, IsothermalSSACAnalysis)):
! 2012             raise SetupError(
  2013                 "Invalid analysis type for Small-Signal AC (SSAC). "
  2014                 "SSAC requires a 'SSACAnalysis' or 'IsothermalSSACAnalysis', "
  2015                 f"but received '{type(self.analysis_spec).__name__}' instead."
  2016             )

@momchil-flex
Copy link
Collaborator

@damiano-flex @momchil-flex I'm now having second thoughts about the current naming of sources and analysis types.

Yeah I don't really know the answer but we must 100% think about it right now before things are released. It does seem to me that the current approach will be becoming more and more tedious/confusing as new regimes are added.

Maybe try to sketch what other analysis we want to support and how what the API might look like - even if it's for one or two years from now. Maybe let's take the discussion off this repo, too.

@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch 2 times, most recently from 0be47a0 to b14df29 Compare October 1, 2025 23:47
SteadyChargeDCAnalysis with SSAC
------------------------------

The :class:`SteadyChargeDCAnalysis` class now supports small signal analysis through the ``ssac_freqs`` parameter:
Copy link
Contributor

@marc-flex marc-flex Oct 2, 2025

Choose a reason for hiding this comment

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

Here I'd remove "now". It seems new today but it won't in a few months?

Best Practices
==============

1. **Ground Reference**: Specify exactly one ground reference using :class:`GroundVoltageSource`. If no ground is specified, the smallest voltage source will be considered as the ground voltage.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't entirely true, right? If no ground is provided, "ground" will be the BC without an array of voltages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right.


1. **Ground Reference**: Specify exactly one ground reference using :class:`GroundVoltageSource`. If no ground is specified, the smallest voltage source will be considered as the ground voltage.

2. **Small Signal Amplitude**: Keep the AC amplitude small (typically 1-10 mV) to ensure linear operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

The only reason why you'd keep the amplitude small is so that the solution has some physical meaning. I think you could say something like this instead: Note that since this is a linear analysis tool, the simulated AC current and voltages will be proportional to the amplitude and may not have physical meaning for large enough amplitudes.

Comment on lines 77 to 82
Common Use Cases
================

- **MOSFET Analysis**: Study transconductance and output conductance vs. frequency.
- **Diode Characterization**: Analyze junction capacitance and conductance.
- **BJT Small-Signal**: Compute h-parameters and frequency response.
Copy link
Contributor

Choose a reason for hiding this comment

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

My only problem with saying this is that the users may expect the corresponding notebook examples, so I would refrain from saying this, at least for now.
what do you think @momchil-flex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that once this is merged, notebooks can be produced with these examples. But in principle, you could already do these things already.

I have a notebook with SSAC that can be released (after few modifications) after this comes out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not necessarily opposed to mentioning use cases before we have an example, but these use cases (apart from the diode characterization) are not our target use cases and we're unlikely to work on an example showcasing our charge solver on a MOSFET for example. The relevant devices for us would be modulators and photodetectors, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Given the other discussion though, this file will go away.

This source represents a small-signal AC excitation defined by a list of bias voltages,
and a small-signal amplitude (linear scale). The list of frequencies of interest must be supplied to the analysis class.
The bias ``voltage`` refers to the DC operating point above the simulation ground. Currently, full circuit simulation though electrical ports is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

full circuit simulation though electrical ports -> full circuit simulation through electrical ports?

Comment on lines 13 to 22
class ACVoltageSource(Tidy3dBaseModel):
"""
Small-signal AC voltage source.
Notes
-----
This source represents a small-signal AC excitation defined by a list of bias voltages,
and a small-signal amplitude (linear scale). The list of frequencies of interest must be supplied to the analysis class.
The bias ``voltage`` refers to the DC operating point above the simulation ground. Currently, full circuit simulation though electrical ports is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is a little tricky. Right now this class is called ACVoltageSource which in principle could be used for either linear or non-linear analysis but everything in this class is done assuming SSAC. So I guess we have two options:

  1. We change the name to something like SSACVoltageSource or
  2. we keep the ACVoltageSource as pure AC source but we treat it as AC since at the moment it can only be defined in simulations where ssac_freq has been defined in the steady analysis spec

The problem with 2 is that in the long term we may want to support AC sources with arbitrary shapes so we'd need to modify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could discuss API alternatives in the Lark document. Each approach has his own strength and weakness.

-----
This establishes the ground reference for the simulation. All other voltage sources
will be referenced to this ground potential. If no GroundVoltageSource is specified,
the smallest voltage among all sources becomes the ground reference.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment to above. The ground (or reference) is the one without voltage arrays?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is actually true. I was just thinking about the case with one single BC for each contact.

Comment on lines 671 to 682
analysis_spec = values.get("analysis_spec")
if (
analysis_spec
and hasattr(analysis_spec, "ssac_freqs")
and len(analysis_spec.ssac_freqs) > 0
):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in general we kind of avoid hasattr though maybe @momchil-flex can comment.
As an alternative you could replace by a isinstance if?

if isinstance(analysis_spec, SteadyChargeAnalysis) and len(analysis_spec.ssac_freqs) > 0:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, writing it in this more manifest way is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the new version implements already the Marc's approach.

if isinstance(bc.condition, VoltageBC):
if isinstance(bc.condition.source, ACVoltageSource):
return (
bc.condition.source.freqs.tolist(),
Copy link
Contributor

Choose a reason for hiding this comment

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

ACVoltageSource no longer has frequencies, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right.

@@ -0,0 +1,89 @@
Small Signal Analysis Guide |:zap:|
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this file. However, it is not in the right place. Our lectures page just links to the video seminar series that we have released. So far, we use a combination of the API reference and the main notebook introducing a given feature to provide the information that you have compiled here (e.g. the HeatChargeSimulation reference and the original charge solver notebook.

Again, I like the idea of this file, and it could be nice if some day we have such "feature white papers". But given that it's a loner right now, probably this info should be moved to the components and the upcoming example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. OK, I'll create an example directly into HeatChargeSimulation, and will save the rest of the information for the upcoming notebook.

Copy link
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Some initial comments. But I think we should consider @daquinteroflex 's proposal in the Lark document, if not implementing fully then at least some aspect of it. I'll put down some more thoughts a bit later.

amplitude: pd.FiniteFloat = pd.Field(
default=1.0,
title="Small-Signal Amplitude",
description="Small-signal AC amplitude. If not specified, it is assumed to be unitary.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you mean unity?

Should we default to a small value like 0.001 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unitary does not look wrong to me, as it is an adjective that means "of or relating to a unit". But to be fair I just searched it now 😃. I just wrote it assuming it to be correct to me. But can switch to "unity" if you don't like it.

Yeah, we could actually default to 1mV.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be honest you could just drop the sentence? The default is the parsed docstring so no need to say anything.

My problem with unitary is that to me it sounds like a unitary matrix, or related to that a unitary time evolution, which has a completely unrelated meaning (but e.g. in some contexts unitary kind of means "lossless", so just want to avoid confusion).

name: Optional[str] = pd.Field(
None,
title="Name",
description="Unique name for the DC current source",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we validate that the names are unique?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, since names are not used anywhere basically. They're just introduced for future (probably near) future. This said, validating the uniqueness regardless seems a good idea to me.

if isinstance(bc.condition, VoltageBC):
if isinstance(bc.condition.source, ACVoltageSource):
if ssac_present:
raise SetupError("Only a single AC source can be supplied")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing period at the end of the message

raise SetupError(
"Frequency list for SSAC analysis is missing. "
"Please specify the frequencies to be analyzed, in the 'analysis_spec.ssac_freqs' attribute."
)
Copy link
Collaborator

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 all the error checking here. Shouldn't these things have been caught already? I guess it doesn't hurt to have extra checks, but if any of those are hit then something's wrong with the initial validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a very good point linked to the API discussion (I hate it sounds like a Gemini response).

Currently, the error error cannot caught earlier because:

  • The AC source is provided as a BC.
  • ssac_freqs is provided to the analysis type (and not to the AC source as in the earlier implementation).

Therefore, the only place where you can have this check is when you assemble the HeatChargeSimulation. Because if there's no AC source, you don't actually need ssac_freqs.

@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch from ee88093 to f7f1d1f Compare October 7, 2025 09:58
@damiano-flex damiano-flex changed the title feat: Add SSAC voltage source feat[FXC-1655]: Add SSAC voltage source Oct 7, 2025
@damiano-flex damiano-flex changed the title feat[FXC-1655]: Add SSAC voltage source feat(FXC-1655): Add SSAC voltage source Oct 7, 2025
@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch 2 times, most recently from 30a8430 to c578465 Compare October 7, 2025 12:38
)

# Two AC sources cannot be defined
with pytest.raises(pd.ValidationError, match="Only a single AC source can be supplied."):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that there's this can test_heat_charge_multiple_ac_sources be removed?

from tidy3d.components.microwave.data.monitor_data import (
AntennaMetricsData,
)
from tidy3d.components.spice.analysis.ac import IsothermalSSACAnalysis
Copy link
Collaborator

Choose a reason for hiding this comment

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

Help me understand how we can also enable SSAC for non-isothermal. Can we do non-isothermal DC and then isothermal SSAC on top of it, and then it is pretty straightforward? Or is it more complicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just requires another class which is a copy of SteadyChargeDCSimulation the same way for isothermal analysis.

Let me add it.

-----
The parameter ``frequency`` is set for future usage,
and is not employed in small-signal analysis
(See :class:`IsothermalSSACAnalysis`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you need a . for this to render correctly if the class name is not in the same file, i.e.

:class:`.IsothermalSSACAnalysis`

With the dot, it will find it in the tidy3d namespace.

Notes
-----
The parameter ``frequency`` is set for future usage,
and is not employed in small-signal analysis
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not talk about future usage. Just say that the frequency parameter is not used in the SSAC.

frequency: Optional[pd.PositiveFloat] = pd.Field(
default=None,
title="Frequency",
description="Frequency of the sinusoidal signal.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"When using SSAC analysis, the value is optional, and even if provided it is overwritten by the 'ssac_freqs' in the analysis specification."

"AC voltage source not found. "
"A Small-Signal AC (SSAC) analysis requires one 'ACVoltageSource' "
"to be defined in the 'boundary_spec', but none was found."
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I'm still confused by these checks here. If the simulation has been correctly initialized and the validators have passed, how can these errors ever be hit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • For the first error, you can still call _get_ssac_frequency_and_amplitude() on a IsothermalDCAnalysis, so you can still hit that error.
  • For the second error, you're right.

Copy link
Contributor

@marc-flex marc-flex left a comment

Choose a reason for hiding this comment

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

I guess the only missing thing is SSAC for non isothermal.

My other concern is the specification of the ACVoltage signal definition. Maybe we should discuss how we go about it with the team

Comment on lines 267 to 270
# Check that frequencies are logarithmically spaced
assert len(freqs) > 0
assert np.isclose(freqs[0], freq_range.fmin)
assert np.isclose(freqs[-1], freq_range.fmax)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you aren't really checking that the frequencies are logarithmically spaced, right? You could for instance check something like this?

for i, f  in enumerate(freqs):
    np.log10(f/freqs[i-1]) == 1/10  # 10 points per decade
np.log10(ffreq_rage.fmax) - np.log10(ffreq_rage.fmin)  == 3  # 3 decades

then the rest of assers below wouldn't be necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the test you supplied, if fmax and fmin were translated (let's say multiplied both by 10), that test would still pass. So better to have a test that checks the actual bounds of fmin and fmax.

Regarding the logarithmic spacing you are right, probably I accidentally removed it at some point. Let me add it back.

Comment on lines 292 to 301
freqs = freq_range.sweep_decade(5)
assert len(freqs) >= 5 # Should have at least 5 points
assert np.isclose(freqs[0], freq_range.fmin)
assert np.isclose(freqs[-1], freq_range.fmax)
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the above comment

Comment on lines 973 to 966
sim = td.HeatChargeSimulation(
size=(8, 8, 8),
center=(0, 0, 0),
structures=structures,
boundary_spec=[
td.HeatChargeBoundarySpec(
placement=td.StructureStructureInterface(structures=["anode", "silicon"]),
condition=td.VoltageBC(source=ac_source),
),
td.HeatChargeBoundarySpec(
placement=td.StructureStructureInterface(structures=["cathode", "silicon"]),
condition=td.VoltageBC(source=td.GroundVoltageSource()),
),
],
grid_spec=td.UniformUnstructuredGrid(dl=0.1),
monitors=[volt_monitor],
analysis_spec=isothermal_spec,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can create this simulation object first so that in the test above (line 952, with pytest...) you can simply update the simulation with new BCs. That way we can save a little bit of code

from tidy3d.constants import inf as td_inf


class IsothermalSSACAnalysis(IsothermalSteadyChargeDCAnalysis):
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also include a non isothermal SSAC analysis type, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the class would be an exact copy. I already have it in local now. But since the inheritance graph would look like this.

SteadyChargeDCAnalysis -----> SSACAnalysis
|
|
|
V
IsothermalSteadyChargeDCAnalysis -> IsothermalSSACAnalysis

I couldn't think of a better way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that I think about it again, did we kind of discuss the option of just SteadyChargeAnalysis and IsothermalSteadyChargeAnalysis with ssac_freqs as an optional field - rather than introducing SSAC subtypes? And then deprecating the ones with DC in the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I added it back because your V2-alternative version includes an ad-hoc analysis class. The unification approach was proposed before your V2 alternative in the lark document.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So I am not sure the V2 (alternative) still necessitates it. Just thinking out loud here: is the main difference going to be whether we check things like if isinstance() with if analysis_spec.ssac_freqs is not None?

Anyway, I guess let's just go with the explicit separation. Note that you could introduce an AbstractSSACAnalysis class that adds the ssac_freqs field and then just SSACAnalysis(SteadyChargeDCAnalysis, AbstractSSACAnalysis), IsothermalSSACAnalysis(IsothermalSteadyChargeDCAnalysis, AbstractSSACAnalysis)`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Yes. An ad hoc class only allows you an explicit specification that you're gong to do SSAC. But given the choices we made, we have to keep it. If we remove it, the only way for SteadyChargeDCAnalysis to understand the user wants to do SSAC is because it finds a ACVoltageSource. But if we go to transient, we can keep the same class and just change analysis type.

  2. Fair enough. Let's do like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the reason why we decided to go for different analysis types was so that we were more explicit and avoid more if statements. It also allows us to check for the stuff we need for the simulation in a more concise way. This is, if we have a SSAC type analysis we know we need to look for some sources, we need frequencies, etc; if we have a non isothermal type of simulation, we look for thermal BCs, etc etc

Comment on lines 19 to 21
The parameter ``frequency`` is set for future usage,
and is not employed in small-signal analysis
(See :class:`IsothermalSSACAnalysis`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just mention that in SS analysis this frequency is ignored in favor of the frequencies defined in the analysis spec.


class SinusoidalSignal(Tidy3dBaseModel):
"""
A transient sinusoidal signal.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just Sinusoidal signal in the time domain?

Comment on lines 64 to 66
class ACVoltageSource(Tidy3dBaseModel):
"""
Small-signal AC voltage source.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't SS-specific, right? We should be able to use this in non linear transient, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not. I can remove SS to be more generic.

Comment on lines 94 to 55
voltage: ArrayFloat1D = pd.Field(
...,
title="DC Bias Voltages",
description="List of DC operating point voltages (above ground) used with :class:`VoltageBC`.",
units=VOLT,
)

signal: ACSignalType = pd.Field(
default=SinusoidalSignal(),
title="Small-signal properties",
description="AC signal specification for small-signal analysis.",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't feel very generic because this is restricting this AC source to a single signal. This is fine for SS but may be troublesome for transient simulations.

I think we should have instead a tuple of signals. In that case, voltages would come from the time-average of each signal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering that ACSignalType is yet not developed, we could create classes that include different signal components inside to convoy the idea that the signal is 1 composed by many components.

However, since it is not completely clear to me what transient AC simulations with multiple signals will look like, I will defer this to your judgment.

@momchil-flex what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. In transient simulation it may not make sense to have an array of signals. So maybe we could accept an array of signals and for transient we could check that only on signal is provided per ACVoltage

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure, presumably if we provide a few common signal types, it should cover almost all ground. We could also provide a CustomSignal with arbitrary time dependence for the ultimate freedom? Might be simpler than allowing for multiple sources @marc-flex ?

Copy link
Contributor

Choose a reason for hiding this comment

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

A CustomSignal can definitely be useful in full unsteady non-linear transient simulations. What "feels" unnatural is to define voltages outside of the signal which is equivalent to separate time-average and oscillating part (assuming is time-periodic).

Having said that, maybe it is OK to have voltage as an optional parameter for the Steady and SSAC cases? It is definitely convenient

@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch 2 times, most recently from a01847d to 6bb6718 Compare October 7, 2025 15:45
@damiano-flex
Copy link
Contributor Author

@greptile check this PR again.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Looking at this PR, I can see it adds Small Signal AC (SSAC) voltage source functionality to the Tidy3D simulation framework. Let me review the key changes:

Overall Assessment

This is a well-structured PR that follows established patterns in the codebase. The implementation adds:

  1. New ACVoltageSource class for AC voltage sources with DC bias and small-signal amplitude
  2. SinusoidalSignal class for signal specification
  3. SSAC analysis classes (SSACAnalysis and IsothermalSSACAnalysis)
  4. Comprehensive validation to ensure proper usage
  5. Test coverage for the new functionality

Key Strengths

  1. Consistent Architecture: The new classes follow the same patterns as existing DCVoltageSource and analysis classes
  2. Proper Validation: Multiple validators ensure:
    • Only one AC source per simulation
    • Only one ground reference
    • Finite voltage values
    • Positive frequencies
    • AC source required when SSAC frequencies are specified
  3. Good Documentation: Classes include proper docstrings and examples
  4. Test Coverage: Comprehensive tests covering validation scenarios

Minor Issues Found

  1. Schema Inconsistency: In the JSON schemas, the VoltageBC.source field changed from allOf to anyOf, which is correct for union types but should be verified for backward compatibility.

  2. Frequency Validation: The sweep_decade method in FreqRange could potentially generate very large arrays for wide frequency ranges - consider adding a safety check.

  3. Documentation: The constraint that only one AC source is allowed could be better documented in the class docstrings.

Code Quality

The implementation demonstrates good software engineering practices:

  • Type annotations are consistent
  • Validation is comprehensive
  • Error messages are clear and helpful
  • The code follows existing patterns

Safety for Merge

This PR is safe to merge because:

  • It's purely additive (no breaking changes to existing functionality)
  • Comprehensive validation prevents invalid configurations
  • Well-tested with both positive and negative test cases
  • Follows established architectural patterns

The confidence score of 4/5 from the Greptile summary is appropriate - this is a solid implementation with minimal risk.

@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch 2 times, most recently from 9e75675 to 8ea620d Compare October 9, 2025 08:41
@damiano-flex damiano-flex force-pushed the damiano/ssac_analysis branch from 8ea620d to c3af265 Compare October 9, 2025 08:44
@damiano-flex
Copy link
Contributor Author

@greptile

The PR was modified completely due to a change of API structure. So, rewrite the PR body with the correct information referring to the updated code.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Looking at the diff and the changes made, I'll rewrite the PR body to accurately reflect the actual implementation:

Greptile Summary

Updated On: 2025-09-10 09:22:23 UTC

This PR adds comprehensive support for Small Signal AC (SSAC) analysis to the Tidy3D simulation framework. The implementation introduces new classes and analysis capabilities that enable frequency-domain small-signal analysis in heat-charge simulations.

Key Changes

1. New SSAC Voltage Source (tidy3d/components/spice/sources/ac.py)

  • Added SSACVoltageSource class for AC analysis with DC bias voltage, small-signal amplitude, and optional naming
  • Includes proper validation to ensure finite voltages and amplitudes
  • Supports array-based DC bias voltages for parametric sweeps

2. Ground Reference Source (tidy3d/components/spice/sources/dc.py)

  • Added GroundVoltageSource class for explicit 0V ground reference
  • Enhanced DCVoltageSource and DCCurrentSource with proper name field validation
  • Provides clearer ground referencing semantics

3. SSAC Analysis Classes (tidy3d/components/spice/analysis/ac.py)

  • Added abstract AbstractSSACAnalysis base class with ssac_freqs field and validation
  • Implemented SSACAnalysis and IsothermalSSACAnalysis for temperature-dependent and independent AC analysis
  • Includes frequency validation (positive, finite frequencies only)

4. Enhanced Frequency Generation (tidy3d/components/frequencies.py)

  • Added sweep_decade() method to FreqRange class for logarithmic frequency sweeps
  • Mimics SPICE AC analysis decade sweep functionality
  • Enables easy generation of frequencies with specified points per decade

5. Enhanced Data Arrays (tidy3d/components/data/data_array.py)

  • Added FreqVoltageDataArray for frequency-voltage domain data
  • Updated simulation data classes to support AC current-voltage characteristics

6. Comprehensive Validation (tidy3d/components/tcad/simulation/heat_charge.py)

  • Added validation to ensure only one SSACVoltageSource per simulation
  • Added validation to ensure only one GroundVoltageSource per simulation
  • Added validation requiring at least one SSACVoltageSource when ssac_freqs is specified
  • Enhanced capacitance monitor validation to support SSAC voltage arrays
  • Added helper methods _get_charge_type() and _get_ssac_frequency_and_amplitude()

7. Updated Type System

  • Updated VoltageSourceType to include all three voltage source types
  • Updated ElectricalAnalysisType to include SSAC analysis types
  • Enhanced schema definitions for all new classes

8. API Integration

  • Added all new classes to main package exports in tidy3d/__init__.py
  • Updated documentation structure to include SSAC components
  • Added comprehensive test coverage for all new functionality

Technical Implementation Details

The SSAC analysis works by:

  1. Establishing a DC operating point using the voltage array in SSACVoltageSource
  2. Performing small-signal linearization around this operating point
  3. Computing frequency-domain response at frequencies specified in ssac_freqs
  4. Using the amplitude parameter to define the perturbation magnitude

The implementation follows established patterns from existing DC analysis while adding the frequency domain capabilities required for AC analysis.

Testing

Comprehensive test coverage has been added in tests/test_components/test_heat_charge.py and tests/test_components/test_frequencies.py, covering:

  • SSAC voltage source validation
  • Multiple AC source constraint validation
  • Frequency generation and validation
  • Integration with existing simulation framework

Important Files Changed

Changed Files
Filename Score Overview
tidy3d/components/spice/sources/ac.py 5/5 New file implementing SSACVoltageSource with comprehensive validation
tidy3d/components/spice/analysis/ac.py 5/5 New file implementing SSAC analysis classes with frequency validation
tidy3d/components/tcad/simulation/heat_charge.py 4/5 Enhanced with multiple validation rules for SSAC constraints
tidy3d/components/frequencies.py 4/5 Added sweep_decade method for logarithmic frequency generation
tidy3d/components/spice/sources/dc.py 4/5 Added GroundVoltageSource and enhanced existing sources
tests/test_components/test_heat_charge.py 5/5 Comprehensive validation tests for SSAC functionality
tests/test_components/test_frequencies.py 5/5 Tests for decade sweep frequency generation

Confidence score: 5/5

  • This PR is safe to merge with very low risk as it adds comprehensive new functionality without breaking existing behavior
  • Well-structured implementation following established patterns with extensive validation
  • Comprehensive test coverage ensures reliability
  • Clear separation of concerns between DC and AC analysis components

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants