ENH: Introducing local sensitivity analysis #575

Lucas-Prates · 2024-03-11T23:07:32Z

Pull request type

Other (please describe): This PR is a draft to implement sensitivity analysis in RocketPy. It is a work in progress and needs validation.

Checklist

Lint (black rocketpy/ tests/) has passed locally

Current behavior

The Sensitivity Analysis notebook teaches the users how to perform the simulations, plot the distribution
of some flight variables (e.g. apogee), and computes the prediction ellipses for the landing point.

New behavior

Our goal is to take sensitivity analysis even further. Briefly, we attempt to answer the following question: Which parameters would reduce the variability of the variable of interest (e.g. apogee) the most if we measured them with greater precision?

To that end, a bit of theory is developed, check the technical document. What was developed resembles the work of [1], a core reference in sensitivity analysis for engineering. His approach is a global sensitivity analysis with a full model containing interaction terms. Our first implementation considers a local sensitivity analysis using only first-order terms.

A quick and dirty test of the functionality of the SensitivityModel class is provided the "sensitivity_model_usage" notebook. This notebook is currently giving weird results! The linear approximations for the variables are, for some reason I still have to figure out, not good enough. This was not happening at previous experimentations that suggested that this approached worked. I have to look carefully at what is happening, but I did not want to delay the PR.

The concepts are discussed in-depth in the "sensitivity_analysis_parameter_importance" notebook (the notebook was not updated to the new SensitivityModel yet!)

Breaking change

No

Additional information

Technical Document

[1] Sobol, Ilya M. "Global sensitivity indices for nonlinear mathematical models and their Monte Carlo estimates." Mathematics and computers in simulation

codecov · 2024-03-11T23:22:44Z

Codecov Report

Attention: Patch coverage is 7.25191% with 243 lines in your changes are missing coverage. Please review.

Project coverage is 72.01%. Comparing base (209434f) to head (c91a50e).
Report is 1 commits behind head on develop.

Files	Patch %	Lines
rocketpy/sensitivity/sensivity_model.py	7.61%	194 Missing ⚠️
rocketpy/tools.py	3.92%	49 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #575      +/-   ##
===========================================
- Coverage    73.60%   72.01%   -1.60%     
===========================================
  Files           70       72       +2     
  Lines        10310    10572     +262     
===========================================
+ Hits          7589     7613      +24     
- Misses        2721     2959     +238

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…t errors

…stall to setup.py

…les, and folders

…imulation's.

… weird results)

Gui-FernandesBR · 2024-05-15T13:17:28Z

Tests are not passing

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/runner/work/RocketPy/RocketPy/rocketpy/__init__.py", line 2, in <module>
    from .environment import Environment, EnvironmentAnalysis
  File "/home/runner/work/RocketPy/RocketPy/rocketpy/environment/__init__.py", line 1, in <module>
    from .environment import Environment
  File "/home/runner/work/RocketPy/RocketPy/rocketpy/environment/environment.py", line [16](https://github.com/RocketPy-Team/RocketPy/actions/runs/9096367936/job/25001616031?pr=575#step:6:17), in <module>
    from ..tools import exponential_backoff
  File "/home/runner/work/RocketPy/RocketPy/rocketpy/tools.py", line 556, in <module>
    parameters_list: list[str],
TypeError: 'type' object is not subscriptable
Error: Process completed with exit code 1.

Could you fix it before our review, please? That would help us. @Lucas-Prates

Lucas-Prates · 2024-05-15T17:07:02Z

Could you fix it before our review, please? That would help us. @Lucas-Prates

Sure, I will fix it briefly. This simplified type hinting started at python 3.9. I will make sure the tests pass this time. :P

Gui-FernandesBR · 2024-05-15T18:30:07Z

Please be aware of #444, we are not supporting type hinting or annotations yet.

Gui-FernandesBR · 2024-05-25T17:19:35Z

setup.py

Why adding a setup.py file?

We are using the pyproject.toml file now. We no longer support the setup.py

MateusStano · 2024-05-27T15:29:32Z

rocketpy/tools.py

+ Parameters
+ ----------
+ input_filename : str
+ Input file exported by MonteCarlo class. Each line is a
+ sample unit described by a dictionary where keys are parameters names
+ and the values are the sampled parameters values.
+
+ output_filename : str
+ Output file exported by MonteCarlo.simulate function. Each line is a
+ sample unit described by a dictionary where keys are target variables
+ names and the values are the obtained values from the flight simulation.
+
+ parameters_list : list[str]
+ List of parameters whose values will be extracted.
+
+ target_variables_list : list[str]
+ List of target variables whose values will be extracted.
+


Suggested change

Parameters

----------

input_filename : str

Input file exported by MonteCarlo class. Each line is a

sample unit described by a dictionary where keys are parameters names

and the values are the sampled parameters values.

output_filename : str

Output file exported by MonteCarlo.simulate function. Each line is a

sample unit described by a dictionary where keys are target variables

names and the values are the obtained values from the flight simulation.

parameters_list : list[str]

List of parameters whose values will be extracted.

target_variables_list : list[str]

List of target variables whose values will be extracted.

Parameters

----------

input_filename : str

Input file exported by MonteCarlo class. Each line is a

sample unit described by a dictionary where keys are parameters names

and the values are the sampled parameters values.

output_filename : str

Output file exported by MonteCarlo.simulate function. Each line is a

sample unit described by a dictionary where keys are target variables

names and the values are the obtained values from the flight simulation.

parameters_list : list[str]

List of parameters whose values will be extracted.

target_variables_list : list[str]

List of target variables whose values will be extracted.

Can't have these line skips here. Breaks docs pages

This also happens in a lot of the other functions in this PR

MateusStano · 2024-05-27T15:30:33Z

rocketpy/tools.py

+ """Reads MonteCarlo simulation data file and builds parameters and flight
+ variables matrices from specified


"from specified..."

Docs not finished here

MateusStano · 2024-05-27T15:33:56Z

rocketpy/tools.py

+ Returns
+ -------
+ parameters_matrix: np.matrix
+ Numpy matrix contaning input parameters values. Each column correspond
+ to a parameter in the same order specified by 'parameters_list' input.
+
+ target_variables_matrix: np.matrix
+ Numpy matrix contaning target variables values. Each column correspond
+ to a target variable in the same order specified by 'target_variables_list'
+ input.


Suggested change

Returns

-------

parameters_matrix: np.matrix

Numpy matrix contaning input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix contaning target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

Returns

-------

parameters_matrix: np.matrix

Numpy matrix contaning input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix contaning target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

Typo! containing

I recoomend the installation of this extension: https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker

Suggested change

Returns

-------

parameters_matrix: np.matrix

Numpy matrix contaning input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix contaning target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

Returns

-------

parameters_matrix: np.matrix

Numpy matrix containing input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix containing target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

MateusStano · 2024-05-27T15:57:05Z

rocketpy/sensitivity/sensivity_model.py

+ def __init__(
+ self,
+ parameters_names,
+ target_variables_names,
+ ):


MateusStano · 2024-05-27T15:59:48Z

rocketpy/sensitivity/sensivity_model.py

+ def fit(
+ self,
+ parameters_matrix,
+ target_data,
+ ):
+ """Fits sensitivity model


Is there any use case where you would define a SensitivityModel and not call .fit() immediately afterward?

This seems like something that could/should be inside __init__

MateusStano · 2024-05-27T16:09:58Z

rocketpy/sensitivity/sensivity_model.py

+ def set_parameters_nominal(
+ self,
+ parameters_nominal_mean,
+ parameters_nominal_sd,
+ ):
+ """Set parameters nominal mean and standard deviation
+
+ Parameters
+ ----------
+ parameters_nominal_mean : np.array
+ An array contaning the nominal mean for parameters in the
+ order specified in parameters names at initialization
+ parameters_nominal_sd : np.array
+ An array contaning the nominal standard deviation for
+ parameters in the order specified in parameters names at
+ initialization
+ """


So do you have to set the mean and sd simultaneously?

Also, do you have to set mean and sd of all parameters? Setting for just some of them does not work?

Another thing, to run a Monte Carlo sim, the mean and sd is already given in the Monte Carlo class right? So it would be natural to get them from there automatically

MateusStano · 2024-05-27T16:12:50Z

rocketpy/sensitivity/sensivity_model.py

+ if parameters_matrix.shape[1] != self.n_parameters:
+ raise ValueError(
+ "Number of columns (parameters) does not match number of parameters passed at initialization."
+ )


Hasn't this already been checked if there is a parameters_matrix ?

Gui-FernandesBR

This is just a partial review. I could not run your codes to validate it is working, but I suggested a few changes that might improve the quality (i.e. readability) of the code.

Gui-FernandesBR · 2024-05-30T19:40:01Z

rocketpy/tools.py

@@ -19,6 +19,7 @@
 from cftime import num2pydate
 from matplotlib.patches import Ellipse
 from packaging import version as packaging_version
+import json


Remember to run isort before pushing to origin. You can you the command make isort in the root directory. THis will ensure the imports are conventionally organized.

Gui-FernandesBR · 2024-05-30T19:44:25Z

rocketpy/tools.py

+ Returns
+ -------
+ parameters_matrix: np.matrix
+ Numpy matrix contaning input parameters values. Each column correspond
+ to a parameter in the same order specified by 'parameters_list' input.
+
+ target_variables_matrix: np.matrix
+ Numpy matrix contaning target variables values. Each column correspond
+ to a target variable in the same order specified by 'target_variables_list'
+ input.


Typo! containing

I recoomend the installation of this extension: https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker

Suggested change

Returns

-------

parameters_matrix: np.matrix

Numpy matrix contaning input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix contaning target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

Returns

-------

parameters_matrix: np.matrix

Numpy matrix containing input parameters values. Each column correspond

to a parameter in the same order specified by 'parameters_list' input.

target_variables_matrix: np.matrix

Numpy matrix containing target variables values. Each column correspond

to a target variable in the same order specified by 'target_variables_list'

input.

Gui-FernandesBR · 2024-05-30T19:48:30Z

rocketpy/tools.py

+ for i in range(n_parameters):
+ parameter = parameters_list[i]
+ parameters_matrix[:, i] = parameters_samples[parameter]
+
+ for i in range(n_variables):
+ target_variable = target_variables_list[i]
+ target_variables_matrix[:, i] = target_variables_samples[target_variable]


Using enumerate is a more pythonic and readable solution.

Suggested change

for i in range(n_parameters):

parameter = parameters_list[i]

parameters_matrix[:, i] = parameters_samples[parameter]

for i in range(n_variables):

target_variable = target_variables_list[i]

target_variables_matrix[:, i] = target_variables_samples[target_variable]

for i, parameter in enumerate(parameters_list):

parameters_matrix[:, i] = parameters_samples[parameter]

for i, target_variable in enumerate(target_variables_list):

target_variables_matrix[:, i] = target_variables_samples[target_variable]

Gui-FernandesBR · 2024-05-30T19:49:12Z

rocketpy/tools.py

+
+ if number_of_samples_parameters != number_of_samples_variables:
+ raise ValueError(
+ "Number of samples for parameters does not match the number of samples for target variables!"


This line is too long (over 88 columns), please break it into 2 lines.

Suggested change

"Number of samples for parameters does not match the number of samples for target variables!"

"Number of samples for parameters does not match the "

"number of samples for target variables!"

Gui-FernandesBR · 2024-05-30T19:52:34Z

rocketpy/tools.py

+ except Exception:
+ raise Exception(
+ f"Variable {variable} was not found in {output_filename}!"
+ )


Your are using a broad exception catch although you are printing a message that is more related to a particular case of exceptions: the KeyError.

Suggested change

except Exception:

raise Exception(

f"Variable {variable} was not found in {output_filename}!"

)

except KeyError as e:

raise KeyError(

f"Variable {variable} was not found in {output_filename}!"

) from e

Gui-FernandesBR · 2024-05-30T20:35:08Z

rocketpy/tools.py

+ # Auxiliary function that unnests dictionary
+ def unnest_dict(x):
+ new_dict = {}
+ for key, value in x.items():
+ # the nested dictionary is inside a list
+ if isinstance(x[key], list):
+ # sometimes the object inside the list is another list
+ # we must skip these cases
+ if isinstance(value[0], dict):
+ inner_dict = unnest_dict(value[0])
+ inner_dict = {
+ key + "_" + inner_key: inner_value
+ for inner_key, inner_value in inner_dict.items()
+ }
+ new_dict.update(inner_dict)
+ else:
+ new_dict.update({key: value})
+
+ return new_dict


I would define this function outside the load_monte_carlo_data() function, this would allow us to re-use this function in other contexts.

Also, the term flatten is usually used to describe this kid of "parse a nested dictionary" operation. Maybe it would be a good alternative of name.

Gui-FernandesBR · 2024-05-30T20:35:32Z

rocketpy/__init__.py

@@ -49,3 +49,5 @@
 StochasticTail,
 StochasticTrapezoidalFins,
 )
+from .sensitivity import SensitivityModel


Gui-FernandesBR · 2024-05-30T20:39:50Z

rocketpy/sensitivity/sensivity_model.py

+
+ References
+ ----------
+ [1] Sobol, Ilya M. "Global sensitivity indices for nonlinear mathematical models and their Monte Carlo estimates." Mathematics and computers in simulation 55.1-3 (2001): 271-280.


Line too long... You can use the \ sign to continue the text in the next line whithout breaking it. Like this:

"something here
continues here, do you see?"

Gui-FernandesBR · 2024-05-30T20:41:47Z

rocketpy/sensitivity/sensivity_model.py

+ """
+ if len(parameters_nominal_mean) != self.n_parameters:
+ raise ValueError(
+ "Nominal mean array length does not match number of parameters passed at initilization."


typo fix. And line is too long.

Suggested change

"Nominal mean array length does not match number of parameters passed at initilization."

"Nominal mean array length does not match number of parameters passed at initialization."

(same thing happens in a few lines below)

Gui-FernandesBR · 2024-05-30T20:42:32Z

rocketpy/sensitivity/sensivity_model.py

+ return
+


You can probally remove this return and the code you keep working.

Suggested change

return

Lucas-Prates requested a review from a team as a code owner March 11, 2024 23:07

github-actions bot assigned Lucas-Prates Mar 11, 2024

Gui-FernandesBR marked this pull request as draft April 4, 2024 04:33

Lucas-Prates added 13 commits May 14, 2024 18:36

ENH: introducing ImportanceModel class for parameter importance analysis

889daf2

MNT: adding imports and renaming analysis folder

be049b5

ENH: implementing plot to ImportanceModel and fixing estimation/impor…

61e8ceb

…t errors

ENH: implementing summary method to ImportanceModel

fd9e052

MNT: adding optional requirements for ImportanceModel

dff90da

MNT: using optional import tools and adding sensitivity dependency in…

665991b

…stall to setup.py

MNT: renaming the term 'importance' to 'sensitivity' in variables, fi…

7fac15e

…les, and folders

MNT: completing renaming from 'importance' to 'sensitivity'

5667784

MNT: Improving doc and input validation.

84e4a11

MNT: fixing plot and input validation in SensitivityModel.

9de87ee

ENH: implementing function in tools to extract data from MonteCarlo s…

429ae75

…imulation's.

DOC: providing a notebook for quick testing of SensitivityModel (with…

a2a7238

… weird results)

MNT: adding json dependency to tools.py

1c7f84d

Lucas-Prates force-pushed the enh/sensitivity_analysis branch from 58b1cdf to 1c7f84d Compare May 14, 2024 21:45

Fix code style issues with Black

ef0ae7e

Lucas-Prates changed the base branch from develop to enh/class_dispersion May 14, 2024 21:46

Lucas-Prates added Enhancement New feature or request, including adjustments in current codes Monte Carlo Monte Carlo and related contents labels May 14, 2024

Lucas-Prates changed the title ~~ENH: (DRAFT) Introducing Parameter Importance in Sensitivity Analysis~~ ENH: Introducing local sensitivity analysis May 14, 2024

Lucas-Prates requested review from Gui-FernandesBR and phmbressan May 14, 2024 22:03

Gui-FernandesBR marked this pull request as ready for review May 15, 2024 13:14

Gui-FernandesBR added this to the Release v1.X.0 milestone May 15, 2024

Gui-FernandesBR linked an issue May 15, 2024 that may be closed by this pull request

Sensitivity Analysis on Monte Carlo Simulations #200

Open

MNT: removing type hints for consistency with codebase (#444)

b09d891

Base automatically changed from enh/class_dispersion to develop May 21, 2024 22:52

Merge branch 'develop' into enh/sensitivity_analysis

c91a50e

Gui-FernandesBR requested a review from MateusStano May 25, 2024 17:17

Gui-FernandesBR reviewed May 25, 2024

View reviewed changes

setup.py

Copy link

Member

Gui-FernandesBR May 25, 2024

Choose a reason for hiding this comment

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

Why adding a setup.py file?

We are using the pyproject.toml file now. We no longer support the setup.py

MateusStano requested changes May 27, 2024

View reviewed changes

Gui-FernandesBR requested changes May 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Introducing local sensitivity analysis #575

ENH: Introducing local sensitivity analysis #575

Lucas-Prates commented Mar 11, 2024 •

edited

codecov bot commented Mar 11, 2024 •

edited

Gui-FernandesBR commented May 15, 2024

Lucas-Prates commented May 15, 2024 •

edited

Gui-FernandesBR commented May 15, 2024

Gui-FernandesBR May 25, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

Gui-FernandesBR May 30, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

MateusStano May 27, 2024

Gui-FernandesBR left a comment

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

Gui-FernandesBR May 30, 2024

		"""Reads MonteCarlo simulation data file and builds parameters and flight
		variables matrices from specified

	"Number of samples for parameters does not match the number of samples for target variables!"
	"Number of samples for parameters does not match the "
	"number of samples for target variables!"

	"Nominal mean array length does not match number of parameters passed at initilization."
	"Nominal mean array length does not match number of parameters passed at initialization."

ENH: Introducing local sensitivity analysis #575

Are you sure you want to change the base?

ENH: Introducing local sensitivity analysis #575

Conversation

Lucas-Prates commented Mar 11, 2024 • edited

Pull request type

Checklist

Current behavior

New behavior

Breaking change

Additional information

codecov bot commented Mar 11, 2024 • edited

Codecov Report

Gui-FernandesBR commented May 15, 2024

Lucas-Prates commented May 15, 2024 • edited

Gui-FernandesBR commented May 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gui-FernandesBR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lucas-Prates commented Mar 11, 2024 •

edited

codecov bot commented Mar 11, 2024 •

edited

Lucas-Prates commented May 15, 2024 •

edited