Fix PINNRepresentation type stability and add docstrings#1039
Open
ajatshatru01 wants to merge 1 commit intoSciML:masterfrom
Open
Fix PINNRepresentation type stability and add docstrings#1039ajatshatru01 wants to merge 1 commit intoSciML:masterfrom
ajatshatru01 wants to merge 1 commit intoSciML:masterfrom
Conversation
Member
|
Probably unnecessary as being removed by the next parser |
Author
ah ok, works for me |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Context / Background
This PR addresses the type instability issue detailed in #932. Specifically, PINNRepresentation contained 25 fields strictly typed as ::Any. Because deterministic fields (like depvars and param_estim) were fully dynamically-typed, performance penalties mounted during setup and inner loop execution of the loss functions.
As a first step towards enhancing the ModelingToolkit PINN parsing pipeline (GSoC 2026 prep), ensuring PINNRepresentation handles its types correctly is critical so that downstream symbolic AST transformations have strong guarantees.
Changes Made:-
Concrete Field Typing: Removed ::Any types for 8 deterministic PINNRepresentation fields and provided them with strict type assertions (depvars::Vector{Symbol}, param_estim::Bool, adaloss::AbstractAdaptiveLoss, etc.).
Optimized PINNLossFunctions: Transformed the PINNLossFunctions struct to a @concrete struct utilizing ConcreteStructs.jl (which NeuralPDE.jl already depends on) to permanently eliminate field access dynamic dispatch on the finalized loss functions without exposing massive generic parametric footprints to the user API.
Fixed ??? Docstrings: Documented all fields correctly in both PINNRepresentation and PINNLossFunctions. Also fixed the missing ?? definitions for logger, log_options, and iteration in the PhysicsInformedNN struct.
Dependency Handling: Migrated the AbstractAdaptiveLoss abstract type definition from adaptive_losses.jl into pinn_types.jl to correctly allow struct type boundary declarations during compilation time.
Added pinn_types_stability_tests.jl : Added a standard @testitem that runs an end-to-end PINN discretization check tagged for the nnpde1 suite, serving as a rapid regression smoke-test for inner representation handling.
I ran the smoke test on my local machine to test if everything is correct.

The test ran all fine.
Add any other context about the problem here.