Modifications/Cleanup/Improvements to Time-Stepped Circuit IRDrop example and model implementation #753

jluquin · 2025-12-05T17:56:06Z

Description

Updated / Cleaned time-step IR Drop model functionality; including: ADC quantization feature (and corresponding io parameters) and new input-dependent PCM read noise model (and corresponding io parameters). Further, example 28 was extended to include more comprehensive (though simple) testing of IR Drop model flags/parameters.

Copilot

Pull request overview

This PR enhances the time-stepped IR Drop model implementation with new features including ADC quantization and input-dependent PCM read noise modeling, along with comprehensive example testing and a license migration from MIT to Apache 2.0.

Key Changes

Added ADC quantization feature with configurable frequency parameter to model Current-Controlled Oscillator (CCO) behavior
Implemented input-dependent PCM read noise model with analytical fit coefficients based on integration time and conductance values
Enhanced example 28 to test new IR Drop model parameters including PCM noise and ADC quantization flags
Migrated license headers from MIT to Apache 2.0 across modified files
Refactored _matmul_irdrop method to use trapezoidal integration instead of summation by default

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.

File	Description
src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py	Core implementation of ADC quantization and PCM read noise models; refactored integration logic; updated license header
src/aihwkit/simulator/parameters/io.py	Added new parameters for xdep PCM read noise, ADC quantization, and integration method selection; updated license header and simplified documentation
examples/28_advanced_irdrop.py	Extended example to test new features with multiple model configurations; updated license header

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

Copilot · 2025-12-05T18:08:00Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+            mvm_even_col_down_adc = torch_sum(i_out_nd, dim=-1)
+        else:
+            # Insert a '0th' time step trapz integration
+            i_out_nd = torch.cat( (i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)


[nitpick] Inconsistent spacing in code: There's an extra space after torch.cat( before the opening parenthesis. Consider removing it for consistency: torch.cat((i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)

Suggested change

i_out_nd = torch.cat( (i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)

i_out_nd = torch.cat((i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)

Copilot · 2025-12-05T18:08:00Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+                                            3.07015063e-08, -6.77079241e-08, 1.17763144e-07]
+
+        # First, convert activations/inputs into integer ns units
+        t_integration = torch_mean(torch_abs(input_) / (2*res))


[nitpick] Double space in formatting: 2*res should be 2 * res for consistency with Python style guidelines (PEP 8).

Suggested change

t_integration = torch_mean(torch_abs(input_) / (2*res))

t_integration = torch_mean(torch_abs(input_) / (2 * res))

Copilot · 2025-12-05T18:08:00Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+        if t_integration > 0.:
+            x = torch_log10(t_integration)
+        else:
+            torch.tensor(0., dtype=t_integration.dtype,device=t_integration.device)


Variable x is used in lines 260-271 but when t_integration <= 0, the code creates a tensor without assigning it to x. This should be: x = torch.tensor(0., dtype=t_integration.dtype, device=t_integration.device)

Suggested change

torch.tensor(0., dtype=t_integration.dtype,device=t_integration.device)

x = torch.tensor(0., dtype=t_integration.dtype, device=t_integration.device)

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

Copilot · 2025-12-05T18:08:01Z

src/aihwkit/simulator/parameters/io.py

+    in the case where a capacitor is not fully charged to achieve a
+    pulse/oscillation. This feature is particularly important in
+    capturing the true behavior of SPLIT PWM mode. This feature is
+    implemented within analog_mvm_irdrop_t taken that it requires


Typo in documentation: "taken that" should be "given that" for correct grammar.

Suggested change

implemented within analog_mvm_irdrop_t taken that it requires

implemented within analog_mvm_irdrop_t given that it requires

Copilot · 2025-12-05T18:08:02Z

src/aihwkit/simulator/parameters/io.py

+
+    ir_drop_integration_sum: bool = False
+    """Sets current integration to use summation rather than the default
+    trapezoidal integation method.


Typo in docstring: "integation" should be "integration".

Suggested change

trapezoidal integation method.

trapezoidal integration method.

Copilot · 2025-12-05T18:08:03Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+            mvm_even_col_down_adc = torch_sum(i_out_nd, dim=-1)
+        else:
+            # Insert a '0th' time step trapz integration
+            i_out_nd = torch.cat( (i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)


[nitpick] Inconsistent spacing in code: There's an extra space after torch.cat( before the opening parenthesis. Consider removing it for consistency: torch.cat((i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)

Copilot · 2025-12-05T18:08:03Z

src/aihwkit/simulator/parameters/io.py

-    Note that other noise sources set by
-    :class:`aihwkit.simulator.parameters.inference.WeightModifierParameter`
-    and :class:`aihwkit.inference.noise` will still be applied.
+    Will disregard all other settings in this case.


[nitpick] Documentation clarity reduced: The original documentation mentioned specific classes (WeightModifierParameter and aihwkit.inference.noise) that would still apply noise sources even when is_perfect=True. This information was removed, which may make it unclear to users that some noise sources could still be active. Consider restoring this helpful context.

Suggested change

Will disregard all other settings in this case.

Will disregard all other settings in this case. Note, however, that some noise sources

(such as those from :class:`WeightModifierParameter` and :mod:`aihwkit.inference.noise`)

may still be active even when ``is_perfect=True``.

Copilot · 2025-12-05T18:08:03Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+    @classmethod
+    def _apply_xdep_pcm_read_noise(
+        cls,
+        weight: Tensor,
+        input_: Tensor,
+        io_pars: IOParametersIRDropT,
+        g_converter: SinglePairConductanceConverter,
+        res: float
+    ) -> Tensor:
+        """
+        Applies input-dependent PCM read noise to weights
+        """
+
+        read_noise_scale = io_pars.xdep_pcm_read_noise_scale
+
+        ## Default model analytical-fit coefficients
+        sigma_noise_slope_coefficients = [0.00021926, -0.00187352,
+                                            0.00655714, -0.0146159, 0.02578764]
+
+        sigma_noise_offset_coefficients = [1.11906167e-09, -9.08576764e-09,
+                                            3.07015063e-08, -6.77079241e-08, 1.17763144e-07]
+
+        # First, convert activations/inputs into integer ns units
+        t_integration = torch_mean(torch_abs(input_) / (2*res))
+
+        if t_integration > 0.:
+            x = torch_log10(t_integration)
+        else:
+            torch.tensor(0., dtype=t_integration.dtype,device=t_integration.device)
+
+        # Convert weights into conductance units
+        conductances_lst, params = g_converter.convert_to_conductances(weight)
+        conductances = torch_abs(conductances_lst[0] - conductances_lst[1])  #[uS]
+
+        sig_noise_slope = ( sigma_noise_slope_coefficients[0]*(x**4) +
+                            sigma_noise_slope_coefficients[1]*(x**3) +
+                            sigma_noise_slope_coefficients[2]*(x**2) +
+                            sigma_noise_slope_coefficients[3]*(x**1) +
+                            sigma_noise_slope_coefficients[4]
+                        )
+        sig_noise_offset = ( sigma_noise_offset_coefficients[0]*(x**4) +
+                            sigma_noise_offset_coefficients[1]*(x**3) +
+                            sigma_noise_offset_coefficients[2]*(x**2) +
+                            sigma_noise_offset_coefficients[3]*(x**1) +
+                            sigma_noise_offset_coefficients[4]
+                        ) * 1e6 #[uS]
+        sig_noise = (sig_noise_slope * conductances) + sig_noise_offset
+        g_final = conductances + read_noise_scale * sig_noise * randn_like(weight)
+
+        # Turn conductances back into unitless weights with original sign preserved
+        weight = g_final * (sign(weight)) / params['scale_ratio']
+
+        return weight


New method _apply_xdep_pcm_read_noise lacks test coverage. Consider adding unit tests to verify the PCM read noise model with various input conditions, especially edge cases like zero integration time and different conductance values.

Copilot · 2025-12-05T18:08:03Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+    @classmethod
+    def _apply_adc_quantization(
+        cls,
+        mvm: Tensor,
+        io_pars: IOParametersIRDropT,
+    ) -> Tensor:
+        """
+        Applies ADC Quantization
+        """
+
+        adc_ticks_even = trunc((mvm / 100) * io_pars.adc_frequency)
+        mvm = adc_ticks_even * 100 / io_pars.adc_frequency
+
+        return mvm


New method _apply_adc_quantization lacks test coverage. Consider adding unit tests to verify ADC quantization behavior with different frequency values and input ranges.

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@ccc-login2.pok.ibm.com>

…on; signoff included Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@ccc-login2.pok.ibm.com> Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc585.pok.ibm.com>

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc585.pok.ibm.com>

charlesmackin · 2025-12-17T20:12:59Z

examples/28_advanced_irdrop.py


 # (C) Copyright 2020, 2021, 2022, 2023, 2024 IBM. All Rights Reserved.
 #
-# Licensed under the MIT license. See LICENSE file in the project root for details.


Please change the license lines to the following license statement so that it is consistent with the rest of the AIHWKIT:

# (C) Copyright 2020, 2021, 2022, 2023, 2024, 2025 IBM. All Rights Reserved. # # Licensed under the MIT license. See LICENSE file in the project root for details.

charlesmackin · 2025-12-17T20:14:05Z

src/aihwkit/simulator/parameters/io.py


 # (C) Copyright 2020, 2021, 2022, 2023, 2024 IBM. All Rights Reserved.
 #
-# Licensed under the MIT license. See LICENSE file in the project root for details.


Please change the license lines to the following license statement so that it is consistent with the rest of the AIHWKIT:

# (C) Copyright 2020, 2021, 2022, 2023, 2024, 2025 IBM. All Rights Reserved. # # Licensed under the MIT license. See LICENSE file in the project root for details.

charlesmackin · 2025-12-17T21:18:27Z

src/aihwkit/simulator/tiles/analog_mvm_irdrop_t.py

+                mvm_even_col_down_adc = torch_trapz(i_out_nd, dim=-1)
+
+            if io_pars.adc_quantization:
+                print('applying 1')


Please remove this print statement. It appears to be leftover from some debugging

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc715.pok.ibm.com>

charlesmackin requested review from PabloCarmona, atvasilopoulos, charlesmackin and Copilot December 5, 2025 18:02

Copilot started reviewing on behalf of charlesmackin December 5, 2025 18:03 View session

Copilot finished reviewing on behalf of charlesmackin December 5, 2025 18:06

Copilot AI reviewed Dec 5, 2025

View reviewed changes

jluquin force-pushed the jluq/irdrop branch from 7927c80 to f9376ac Compare December 5, 2025 18:28

Jose Luquin jluquin@ibm.com and others added 3 commits December 5, 2025 17:22

Modifications/Cleanup/Improvements to IRDrop example and implementation

dbba926

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@ccc-login2.pok.ibm.com>

Modifications/Cleanup/Improvements to IRDrop example and implementati…

864b090

…on; signoff included Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@ccc-login2.pok.ibm.com> Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc585.pok.ibm.com>

PR modifications and corrections

5e766b3

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc585.pok.ibm.com>

jluquin force-pushed the jluq/irdrop branch from 731231f to 5e766b3 Compare December 5, 2025 23:20

charlesmackin reviewed Dec 17, 2025

View reviewed changes

PR modifications/update

5b59105

Signed-off-by: Jose Luquin jluquin@ibm.com <jluquin@cccxc715.pok.ibm.com>

	i_out_nd = torch.cat( (i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)
	i_out_nd = torch.cat((i_out_nd[...,0].unsqueeze(-1), i_out_nd), -1)

	t_integration = torch_mean(torch_abs(input_) / (2*res))
	t_integration = torch_mean(torch_abs(input_) / (2 * res))

	torch.tensor(0., dtype=t_integration.dtype,device=t_integration.device)
	x = torch.tensor(0., dtype=t_integration.dtype, device=t_integration.device)

	implemented within analog_mvm_irdrop_t taken that it requires
	implemented within analog_mvm_irdrop_t given that it requires

	trapezoidal integation method.
	trapezoidal integration method.

-    Will disregard all other settings in this case.
+    Will disregard all other settings in this case. Note, however, that some noise sources
+    (such as those from :class:`WeightModifierParameter` and :mod:`aihwkit.inference.noise`)
+    may still be active even when ``is_perfect=True``.

Modifications/Cleanup/Improvements to Time-Stepped Circuit IRDrop example and model implementation #753

Are you sure you want to change the base?

Modifications/Cleanup/Improvements to Time-Stepped Circuit IRDrop example and model implementation #753

Uh oh!

Conversation

jluquin commented Dec 5, 2025

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

charlesmackin Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlesmackin Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

charlesmackin Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

charlesmackin Dec 17, 2025 •

edited

Loading