Skip to content

Conversation

@langestefan
Copy link
Member

This will add an extension for ModelingToolkit.jl to enable creation of solar position modeling blocks that can be plugged into any symbolic model.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a ModelingToolkit.jl extension to SolarPosition.jl, enabling integration of solar position calculations into symbolic modeling workflows for applications like solar energy modeling, building thermal analysis, and solar tracking systems.

Key changes:

  • Created SolarPositionModelingToolkitExt extension that provides a SolarPositionBlock component for computing solar azimuth, elevation, and zenith angles as time-varying symbolic variables
  • Added comprehensive tests demonstrating component creation, composition with other systems, and ODE solving
  • Added documentation with examples showing integration with solar panel and building thermal models

Reviewed Changes

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

Show a summary per file
File Description
ext/SolarPositionModelingToolkitExt.jl Implements the ModelingToolkit extension with SolarPositionBlock component and symbolic function registration
src/SolarPosition.jl Exports new types and adds comprehensive docstring for SolarPositionBlock
src/Positioning/Positioning.jl Introduces AbstractApparentSolPos type hierarchy for better type organization
test/test-mtk.jl Adds tests for block creation, composition, and ODE solving with plotting validation
test/runtests.jl Comments out previous imports (follows existing pattern for auto-discovery)
test/Project.toml Adds test dependencies for ModelingToolkit, OrdinaryDiffEq, and CairoMakie
docs/src/examples/modelingtoolkit.md Comprehensive guide with examples of solar panel and building thermal models
docs/src/index.md Documents the new ModelingToolkit extension in the main index
docs/make.jl Adds modelingtoolkit.md to documentation build
docs/Project.toml Adds ModelingToolkit and OrdinaryDiffEq for documentation builds
examples/mtk-integration.jl Commented example code demonstrating various use cases
examples/Project.toml Declares dependencies for examples
Project.toml Adds ModelingToolkit and Symbolics as weak dependencies with compat entries

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

@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.74%. Comparing base (4a17e29) to head (5e1824b).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
src/Positioning/spa.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   92.36%   92.74%   +0.37%     
==========================================
  Files          15       15              
  Lines         511      510       -1     
==========================================
+ Hits          472      473       +1     
+ Misses         39       37       -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

langestefan and others added 4 commits November 21, 2025 14:35
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@langestefan
Copy link
Member Author

@copilot run Julia formatter to pass linting CI

Copilot AI mentioned this pull request Nov 22, 2025
4 tasks
Copy link
Contributor

Copilot AI commented Nov 22, 2025

@langestefan I've opened a new pull request, #17, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 4 commits November 22, 2025 15:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 9 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines 81 to 100
```julia
using SolarPosition, ModelingToolkit
using ModelingToolkit: t_nounits as t
using Dates
@named sun = SolarPositionBlock()
obs = Observer(51.5, -0.18, 15.0)
t0 = DateTime(2024, 6, 21, 12, 0, 0)
sys = mtkcompile(sun)
pmap = [
sys.observer => obs,
sys.t0 => t0,
sys.algorithm => PSA(),
sys.refraction => NoRefraction(),
]
prob = ODEProblem(sys, pmap, (0.0, 86400.0))
sol = solve(prob; saveat = 3600.0)
```
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The example code is incomplete and missing required imports. The code uses @named (line 86) and mtkcompile (line 90) from ModelingToolkit, and ODEProblem and solve (lines 98-99) from OrdinaryDiffEq, but these are not imported. Add the following imports to make the example self-contained:

using SolarPosition, ModelingToolkit
using ModelingToolkit: t_nounits as t, @named, mtkcompile
using Dates
using OrdinaryDiffEq: ODEProblem, solve

Copilot uses AI. Check for mistakes.
using ModelingToolkit: t_nounits as t, D_nounits as D
using Dates: DateTime
using OrdinaryDiffEq
using CairoMakie
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

Missing import for TimeZones functions. The test uses astimezone (line 183) and tz"UTC" (line 183) from TimeZones package but doesn't import TimeZones. Add using TimeZones: astimezone, @tz_str at the top of this file.

Suggested change
using CairoMakie
using CairoMakie
using TimeZones: astimezone, @tz_str

Copilot uses AI. Check for mistakes.
langestefan and others added 2 commits November 23, 2025 21:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@langestefan langestefan merged commit 7a4cb78 into main Nov 23, 2025
13 of 14 checks passed
@langestefan langestefan deleted the mtk-extension branch November 23, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant