- Notifications
You must be signed in to change notification settings - Fork 92
Custom pedestal api #1752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Custom pedestal api #1752
Conversation
This commit implements a Custom Pedestal Model API that allows users to define custom pedestal scaling laws without modifying TORAX source code. Fixes google-deepmind#1711 ## Changes - Add CustomPedestalModel class supporting user-defined callable functions - Add CustomPedestal Pydantic configuration - Update PedestalConfig union to include CustomPedestal - Add comprehensive unit tests (7 test cases) - Add example configuration with EPED-like scaling - Add complete API documentation ## Features Users can now provide Python functions to compute: - Ion temperature at pedestal (T_i_ped) - Electron temperature at pedestal (T_e_ped) - Electron density at pedestal (n_e_ped) - Optional dynamic pedestal location (rho_norm_ped_top) Functions receive full access to runtime parameters, geometry, and core profiles, enabling machine-specific scaling laws (e.g., STEP pedestal models with Europed data fits). ## API Design Follows the transport model pattern with: - JAX Model Layer: CustomPedestalModel (frozen dataclass) - Pydantic Config Layer: CustomPedestal (validation) - Runtime Parameters: time-varying support Fully backwards compatible - no changes to existing models.
- Add public pedestal API (torax/pedestal.py) following transport model pattern - Remove PR documentation files (PR_SUMMARY.md, PR_TEXT.md, CUSTOM_PEDESTAL_API.md) - Add RST documentation (docs/custom_pedestal_models.rst) - concise and properly formatted - Simplify example to single model using public API and StaticDataclass - Fix test imports (removed 'as pm' alias) - Revert PedestalConfig formatting to single line Addresses all review comments from @tamaranorman
- Use public API throughout (Geometry, CoreProfiles, JAX_STATIC from torax) - Remove StaticDataclass from inheritance (parent already has it) - Simplify example config (minimal pedestal-only config) - Add test for custom_pedestal_example in examples_test.py - Clarify in RST docs that config example is minimal Addresses all review comments from @tamaranorman
| register_model.register_pedestal_model(TestPedestal) | ||
| | ||
| # Verify it can be instantiated | ||
| config = TestPedestal(test_value=99.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we need to test is that it can be instantiated through the ModelConfig API not directly which works even without the test
| register_model.register_pedestal_model(Config1) | ||
| register_model.register_pedestal_model(Config2) | ||
| | ||
| # Verify both can be instantiated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a reference to this to init.py
| Can you also fix the broken tests |
…le to __init__.py - Update register_model_test.py to test model instantiation through ToraxConfig.from_dict() instead of direct class instantiation - Add pedestal module import to torax/__init__.py for better API accessibility - Tests now properly verify that registered models work through the pydantic discriminator system Addresses maintainer feedback on PR.
8768c31 to fc93e9f Compare | | ||
| # Verify it can be instantiated through the ModelConfig API | ||
| # Create a minimal ToraxConfig using from_dict to test the registration | ||
| minimal_config_dict = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use the config from test_utils.default_configs here and below
| Please also fix the tests |
No description provided.