Skip to content

Add IST reduction#119

Open
ai-qui wants to merge 18 commits intomainfrom
feature/#97/ist-reduction
Open

Add IST reduction#119
ai-qui wants to merge 18 commits intomainfrom
feature/#97/ist-reduction

Conversation

@ai-qui
Copy link
Copy Markdown
Member

@ai-qui ai-qui commented Mar 2, 2026 •
edited
Loading

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?

Fixes #97

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

@ai-qui ai-qui self-assigned this Mar 2, 2026
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from 444172c to 6fd8edd Compare March 2, 2026 09:13
@ai-qui ai-qui changed the title Feature/#97/ist reduction Add IST reduction Mar 3, 2026
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from 6fd8edd to 0416620 Compare March 3, 2026 08:02
@ai-qui ai-qui changed the base branch from main to feature/#101/wind-angle March 3, 2026 09:29
Base automatically changed from feature/#101/wind-angle to main March 3, 2026 09:37
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from 4a391ec to 2916d3a Compare March 3, 2026 10:02
@ai-qui ai-qui changed the base branch from main to fix-solar-heating March 3, 2026 10:30
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch 4 times, most recently from 3fed054 to be407bd Compare March 3, 2026 11:34
ai-qui
ai-qui commented Mar 3, 2026
View reviewed changes
Comment thread src/thermohl/solver/slv1t.py Outdated
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch 3 times, most recently from d146018 to 31d23ef Compare March 9, 2026 09:46
@ai-qui ai-qui marked this pull request as ready for review March 9, 2026 09:47
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch 2 times, most recently from f5c6440 to c2f5c1d Compare March 9, 2026 15:37
ai-qui
ai-qui commented Mar 19, 2026
View reviewed changes
Comment thread src/thermohl/solver/slv1t.py
adriengoeller
adriengoeller requested changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@adriengoeller adriengoeller left a comment

Choose a reason for hiding this comment

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

Could you add some integration tests based on the prototype to check values for some usecases ?
We can discuss about the usecases

Comment thread src/thermohl/power/convective_cooling.py
wind_attack_angle: floatArrayLike,
) -> None:
# Compute missing wind attack angles
if isinstance(wind_attack_angle, np.ndarray) and wind_attack_angle.ndim > 0:
Copy link
Copy Markdown
Member

@adriengoeller adriengoeller Apr 1, 2026

Choose a reason for hiding this comment

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

I'm not sure but I was expecting to check the size of the input before creating the mask. But it could be an intended behavior (built in numpy raise)

Copy link
Copy Markdown
Member Author

@ai-qui ai-qui Apr 10, 2026

Choose a reason for hiding this comment

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

I'll check the size of the input in _check_arguments

Copy link
Copy Markdown
Member Author

@ai-qui ai-qui Apr 14, 2026

Choose a reason for hiding this comment

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

I've changed my mind, it's easier to check it here

Comment thread src/thermohl/solver/slv1t.py Outdated
Comment thread src/thermohl/solver/slv1t.py
Comment thread src/thermohl/solver/slv1t.py
Comment thread src/thermohl/solver/slv1t.py Outdated
@ai-qui ai-qui changed the base branch from fix-solar-heating to main April 3, 2026 12:17
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from 23f300f to 481cc2a Compare April 3, 2026 12:21
ai-qui added 3 commits April 3, 2026 14:56
@ai-qui
feat(#97): Add IST reduction feature
b6951ff 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
docs: Add a notebook with an example
afd5121 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
docs: Document IST reduction
a5dbd8c 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
ai-qui added 8 commits April 3, 2026 14:57
@ai-qui
Use secant (quasi-newton) method
8f677b5 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Fix some sonarqube errors
bdfd181 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Ignore sonarqube error about cognitive complexity
a85d00f 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Refacto ConvectiveCooling and fix handling of wind_attack_angle
3d04327 
	- Make cigre ConvectiveCooling inherit ConvectiveCoolingBase like
	the others, so as to factor wind attack angle check and
	computations.

	- Handle missing case in wind attack angle computation
	(when it's nan or an array containing nans)

	- Enable orriding wind attack angle when initializing a
	solver

	- Rename _nu_forced -> _nusselt_forced and _nu_natural ->
	_nusselt_natural

Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Various fixes
8fd9602 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Remove dead code and add test
c9ce67e 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Add tests
46c4aa8 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Add type hints
ef18376 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from a7e47cd to ef18376 Compare April 3, 2026 12:59
@ai-qui
Copy link
Copy Markdown
Member Author

ai-qui commented Apr 10, 2026

Could you add some integration tests based on the prototype to check values for some usecases ? We can discuss about the usecases

I remember discussing this with Hervé and if I've understood correctly, the formulae used in the prototype aren't exactly the same as the ones implemented here. We should discuss it with Hervé again.

ai-qui added 5 commits April 10, 2026 10:48
@ai-qui
Formatting + improvements after review
1272311 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Merge remote-tracking branch 'origin/main' into feature/#97/ist-reduc…
3a2c68d 
…tion
@ai-qui
Use logger instead of print
74a55d8 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
Improvements after review
0f5a911 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui
notebooks: Update Ist reduction notebook
8c78c26 
Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 16, 2026

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
89.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@ai-qui
Copy link
Copy Markdown
Member Author

ai-qui commented Apr 16, 2026

Turns out the "solar flow" means SolarHeating.solar_irradiance, not measured_global_radiation. I'll have to fix the code to reflect this.

@ai-qui
Fix handling of solar heating/solar irradiance
f1de9d6 
	The user wants the be able to set the solar irradiance,
	not the measured global radiation.

	Adds new solar heating power term
	`FixedSolarIrradianceSolarHeating` to be able to set
	the solar irradiance instead of computing it.

Signed-off-by: ai-qui <184963772+ai-qui@users.noreply.github.com>
@ai-qui ai-qui force-pushed the feature/#97/ist-reduction branch from b5f6519 to f1de9d6 Compare April 22, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SaintierFr SaintierFr Awaiting requested review from SaintierFr

@jeremy-vernay jeremy-vernay Awaiting requested review from jeremy-vernay

@adriengoeller adriengoeller Awaiting requested review from adriengoeller

Requested changes must be addressed to merge this pull request.

Assignees

@ai-qui ai-qui

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add IST reduction feature

2 participants

@ai-qui @adriengoeller