Skip to content

Restructure package#203

Merged
mcw92 merged 56 commits into
mainfrom
maintenance/restructure_package
May 13, 2026
Merged

Restructure package#203
mcw92 merged 56 commits into
mainfrom
maintenance/restructure_package

Conversation

@mcw92

@mcw92 mcw92 commented May 4, 2026

Copy link
Copy Markdown
Member

Description

This PR introduces a hopefully more reasonable and intuitive package structure:

artist/
├── __init__.py
│
├── field/                              # unchanged
├── scene/                              # unchanged
├── scenario/                           # unchanged
│
├── io/                                 # renamed from data_parser/
│   ├── __init__.py
│   ├── calibration_parser.py           # renamed (was calibration_data_parser.py)
│   ├── paint_calibration_parser.py
│   ├── paint_scenario_parser.py
│   ├── stral_scenario_parser.py
│   └── h5_scenario_parser.py
│
├── nurbs/
│   ├── __init__.py
│   ├── surfaces.py                     # NURBSSurfaces class (was util/nurbs.py)
│   └── utils.py                        # create_planar_nurbs_control_points(), create_nurbs_evaluation_grid() (from util/utils.py)
│
├── raytracing/
│   ├── __init__.py
│   ├── heliostat_ray_tracer.py         # HeliostatRayTracer (from core/)
│   ├── blocking.py                     # blocking/shadowing (from core/)
│   ├── sampling.py                     # DistortionsDataset, RestrictedDistributedSampler (extracted from heliostat_ray_tracer.py)
│   └── geometry.py                     # reflect(), line_plane_intersections(), line_cylinder_intersections() (was util/raytracing_utils.py)
│
├── optimization/
│   ├── __init__.py
│   ├── surface_reconstructor.py
│   ├── kinematics_reconstructor.py
│   ├── motor_position_optimizer.py
│   ├── loss_functions.py
│   ├── regularizers.py
│   └── training.py                     # LR scheduler factories + EarlyStopping (was learning_rate_schedulers.py)
│
├── geometry/
│   ├── __init__.py
│   ├── transforms.py                   # rotate_e/n/u/distortions(), translate_enu(), perform_canting()
│   ├── coordinates.py                  # convert_3d_points/directions_to_4d_format(), normalize_points(), bitmap_coordinates_to_target_coordinates()
│   └── rotations.py                    # decompose_rotations(), rotation_angle_and_axis()
│
├── flux/                               # open to better name suggestions — see note below
│   ├── __init__.py
│   └── bitmap.py                       # get_center_of_mass(), crop_flux_distributions_around_center(), trapezoid_distribution()
│
└── _util/                              # renamed from util/ — infrastructure only
    ├── __init__.py
    ├── constants.py                    # config_dictionary string keys (was config_dictionary.py)
    ├── indices.py                      # tensor dimension indices (was index_mapping.py)
    ├── environment.py                  # get_device(), DdpSetup, distributed init (was environment_setup.py)
    └── type_registry.py                # type_mappings.py

I implemented the structure outlined above as is.
A couple of things need to be discussed before I can finalize this PR:

  • Cline forgot some functions in the restructuring above which I refactored manually into the modules that seemed to be the most suitable for me. This was the case for mean_loss_per_heliostat (now in artist/optimization/loss_functions.py) as well as azimuth_elevation_to_enu and convert_wgs84_coordinates_to_local_enu (now in artist/geometry/coordinates.py). I am not super happy with this but did not find a better fit so @MarleneBusch please pay particular attention to functions that seem to be "misplaced" from your point of view.
  • The __init__.py files of the package itself and its subpackages seemed to be overloaded which introduced new dependencies and caused unnecessary circular imports and long import times. A package's __init__.py should mark the directory as a package and define a clean public API but not wire together the entire project. I basically emptied all __init__.py files for now. We should think about a clean public API for ARTIST, i.e., which stable, top-level concepts we want to expose explictly to users in the style of from artist import Scenario vs. from artist.scenario.scenario import Scenario. Once we decided for a clean public API, we can update the tutorials and documentation accordingly.
  • Check that all scripts in tutorials and examples still work.
  • There was once circular import field <-> scenario which I fixed quick and dirty by using TYPE_CHECKING for type hints and a local imports for the method in artist/scenario/scenario.py. Are we fine with that?

Type of change

Maintenance only, does not affect the functionality.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Name Stmts Miss Cover Missing
artist\__init__.py 3 0 100%
artist\field\__init__.py 14 0 100%
artist\field\actuators.py 16 0 100%
artist\field\actuators_ideal.py 9 0 100%
artist\field\actuators_linear.py 57 0 100%
artist\field\heliostat_field.py 116 0 100%
artist\field\heliostat_group.py 52 0 100%
artist\field\heliostat_group_rigid_body.py 23 0 100%
artist\field\kinematics.py 12 0 100%
artist\field\kinematics_rigid_body.py 67 0 100%
artist\field\solar_tower.py 48 0 100%
artist\field\surface.py 22 0 100%
artist\field\tower_target_areas.py 15 0 100%
artist\field\tower_target_areas_cylindrical.py 40 0 100%
artist\field\tower_target_areas_planar.py 32 0 100%
artist\flux\__init__.py 2 0 100%
artist\flux\bitmap.py 65 1 98% 116
artist\geometry\__init__.py 4 0 100%
artist\geometry\coordinates.py 95 1 99% 290
artist\geometry\rotations.py 31 1 97% 112
artist\geometry\transforms.py 97 1 99% 346
artist\io\__init__.py 3 0 100%
artist\io\calibration_parser.py 42 2 95% 197-202
artist\io\h5_scenario_parser.py 149 5 97% 550, 554, 560, 565, 570
artist\io\paint_calibration_parser.py 66 0 100%
artist\io\paint_scenario_parser.py 193 0 100%
artist\io\stral_scenario_parser.py 42 0 100%
artist\nurbs\__init__.py 3 0 100%
artist\nurbs\surfaces.py 156 20 87% 210-241, 395, 399, 401-402, 553-554, 557-558
artist\nurbs\utils.py 24 0 100%
artist\optim\__init__.py 7 0 100%
artist\optim\kinematics_reconstructor.py 127 16 87% 159, 309-310, 461-471, 526-549
artist\optim\loss.py 76 0 100%
artist\optim\motor_position_optimizer.py 164 13 92% 323-333, 487, 583-590, 648-657
artist\optim\regularizers.py 24 0 100%
artist\optim\surface_reconstructor.py 162 12 93% 531-539, 628-646
artist\optim\training.py 32 1 97% 179
artist\raytracing\__init__.py 4 0 100%
artist\raytracing\blocking.py 283 9 97% 378, 380, 552-555, 592-593, 722, 737, 975
artist\raytracing\geometry.py 91 0 100%
artist\raytracing\heliostat_ray_tracer.py 142 1 99% 176
artist\raytracing\sampling.py 29 0 100%
artist\scenario\__init__.py 4 0 100%
artist\scenario\h5_scenario_generator.py 83 30 64% 117-141, 153-161, 185, 192-197, 215-218
artist\scenario\scenario.py 94 2 98% 215, 434
artist\scenario\surface_generator.py 89 0 100%
artist\scene\__init__.py 5 0 100%
artist\scene\light_source.py 11 0 100%
artist\scene\light_source_array.py 28 0 100%
artist\scene\rays.py 7 0 100%
artist\scene\sun.py 43 0 100%
artist\util\__init__.py 56 18 68% 101-135
artist\util\config.py 203 0 100%
artist\util\constants.py 303 0 100%
artist\util\env.py 110 23 79% 89-91, 186-228, 308
artist\util\indices.py 322 0 100%
artist\util\type_registry.py 11 0 100%
TOTAL 4008 156 96%

@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.96296% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.10%. Comparing base (e2669d2) to head (2b82f9e).

Files with missing lines Patch % Lines
artist/optim/motor_position_optimizer.py 81.25% 3 Missing ⚠️
artist/flux/bitmap.py 98.46% 1 Missing ⚠️
artist/geometry/coordinates.py 98.94% 1 Missing ⚠️
artist/geometry/rotations.py 96.77% 1 Missing ⚠️
artist/geometry/transforms.py 98.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   96.06%   96.10%   +0.04%     
==========================================
  Files          48       57       +9     
  Lines        3961     4008      +47     
==========================================
+ Hits         3805     3852      +47     
  Misses        156      156              

☔ 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.

@mcw92 mcw92 requested a review from MarleneBusch May 4, 2026 20:18
@mcw92 mcw92 self-assigned this May 4, 2026
@mcw92 mcw92 requested a review from kalebphipps May 4, 2026 20:18
@mcw92

mcw92 commented May 7, 2026

Copy link
Copy Markdown
Member Author

As discussed in our last meeting, I adapted all imports to match the new package structure.

  • The __init__ files of each subpackage now bundle the functionality of all submodules contained in the respective subpackage.
  • Internal code (everything in the actual package) always uses full-length import statements (to avoid circular imports), while tests (and tutorials and examples) use shortened imports as a user would probably do.
  • In addition, I renamed some subpackages and modules again (e.g., optimization now is optim) so import statements are less lengthy. I tried to make everything as intuitive and self-explanatory as possible.
  • The dirty fix concerning TYPE_CHECKING and local import to avoid one specific circular import is still there and will probably stay until we detangle the mixing of IO and domain-specific code in the package (likely cause for circular imports). We decided to leave this as is for now and address this in a future PR.

@MarleneBusch (@kalebphipps) The scripts in examples ans tutorials still need to be tested. If you have the time, feel free to do that.
Before marking this PR ready for review, I would like to check everything again (probably on Monday). In the best case, nothing will change so you can already have a "pre-review" look at the code.

@mcw92 mcw92 marked this pull request as ready for review May 11, 2026 19:29
@MarleneBusch

Copy link
Copy Markdown
Member

I have reviewed all changed files of this PR. I only added very minor changes (removed my system specific paths in the examples configs, which I forgot earlier, and one #---- comment line that had no meaning.)

Overall this PR ist really good and I am amazed how much shorter and more concise the code got with this new structure and the revised imports. Thanks for also fixing docstring along the way.

If its up to me this can be merged into the main now.

I am wondering if we should include a short "How to import" in the contribution guidelines? I know that I havent fully understood all of the rules you applied in this PR for the imports (I only know it looks so much better) but I also know some of it was "Common sense" so I am not sure if we can even write down all the rules here.

(I will also open an issue so we can consider renaming the motor pos optimization to aim point optimiaztion, which is not directly linked to this PR but just came back into my mind while reviewing this.)

@mcw92 mcw92 merged commit 5c2dd9b into main May 13, 2026
12 checks passed
@mcw92 mcw92 deleted the maintenance/restructure_package branch May 13, 2026 07:37
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.

2 participants