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.
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.
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.
Aaryan-549
force-pushed
the
custom-pedestal-api
branch
from
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.
Just use the config from test_utils.default_configs here and below
|
Please also fix the tests |
|
@Aaryan-549 are you still working on this PR and planning to make the changes in response to the review comments? |
|
Yes, I am really sorry for the delay. I'll push a PR with all the changes required ASAP. |
…nfig, update IMAS exception type, and add eq=False to pedestal model tests
|
Fixed the test failures and resolved conflicts! |
|
The tests still seem to be failing - can you make sure to run them manually and fix properly before sending again |
|
You seem to have added lots of extra formatting changes - can you remove these and also merge with head? Also make sure you have actually run the tests and they pass before sending back for review |
…nd model state pollution
merge main
Aaryan-549
force-pushed
the
custom-pedestal-api
branch
from
3f5176e to
c605609
Compare
Fix parallel pytest test isolation and model registration
fixed: transport_model_test.py & sim_time_dependence_test.py
|
Hi @tamaranorman, I apologize for the inconvenience caused by the previous code and failing tests. It took some time to resolve the issues, but everything is passing now. The only errors I’m seeing on my end are worker failures due to my laptop running out of memory during the test run. Please let me know if you have any other suggestions! |
3 participants
No description provided.