Skip to content

Improve minimization functionality #151

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.

Sign up for GitHub

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

Merged
AndrewSazonov merged 23 commits into from
Apr 11, 2026
Merged

Improve minimization functionality #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e5c0b7a
Apply latest templates
AndrewSazonov Apr 10, 2026
f9e74a7
Correcting peak profiles
AndrewSazonov Apr 10, 2026
3679536
Make load_numeric_block skip non-numeric lines anywhere in file
AndrewSazonov Apr 10, 2026
d41d873
Add double-jorgensen-von-dreele peak profile (CrysPy type0m)
AndrewSazonov Apr 10, 2026
3a05ab4
Add some temporary documents and tests [ci skip]
AndrewSazonov Apr 10, 2026
a675d5d
Clean up
AndrewSazonov Apr 10, 2026
ffdd5b2
Exclude tmp/ directory and update roadmap
AndrewSazonov Apr 10, 2026
c1707ca
Clean up
AndrewSazonov Apr 10, 2026
40c7e32
Make data_type read-only on experiments
AndrewSazonov Apr 10, 2026
84c265c
Refactor analysis and base modules to simplify API in pseudo-switchab…
AndrewSazonov Apr 10, 2026
b7f8297
Restore minimiser variant support via thin subclasses
AndrewSazonov Apr 10, 2026
438cdc0
Add bumps minimizer
AndrewSazonov Apr 11, 2026
e794551
Use physical limits for the fitted parameters
AndrewSazonov Apr 11, 2026
e6cc024
Colorize correlation matrix: red negative, blue positive
AndrewSazonov Apr 11, 2026
2130146
Update tests for physical limits and correlation colorization
AndrewSazonov Apr 11, 2026
53c5d3b
Fix cell length_a validator upper bound from 3.885 to 30
AndrewSazonov Apr 11, 2026
17f2ab6
Use default minimizer in Analysis constructor
AndrewSazonov Apr 11, 2026
ecdd7c9
Update test expectations for bumps (lm) default minimizer
AndrewSazonov Apr 11, 2026
b709ab2
Add more issues to issues_open.md
AndrewSazonov Apr 11, 2026
dd7c9cb
Merge branch 'develop' into data_type
AndrewSazonov Apr 11, 2026
766ef75
Fix formatting
AndrewSazonov Apr 11, 2026
5d4ed3e
Update some tutorials
AndrewSazonov Apr 11, 2026
cc97a5c
Add SEQUENTIAL to FitModeEnum and show methods to Analysis
AndrewSazonov Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,28 @@

## Workflow

- Use a two-phase workflow for all non-trivial changes:
- **Phase 1 — Implementation:** implement the change (source code,
docs, architecture updates). Do not create new tests or run existing
tests. Present the implementation for review and iterate until
approved.
- **Phase 2 — Verification:** once the implementation is approved, add
or update tests, then run linting (`pixi run fix`, `pixi run check`)
and all test suites (`pixi run unit-tests`,
`pixi run integration-tests`, `pixi run script-tests`).
- All open issues, design questions, and planned improvements are
tracked in `docs/architecture/issues_open.md`, ordered by priority.
When an issue is fully implemented, move it from that file to
`docs/architecture/issues_closed.md`. When the resolution affects the
architecture, update the relevant sections of
`docs/architecture/architecture.md`.
- After changes, run linting and formatting fixes with `pixi run fix`.
Do not check what was auto-fixed, just accept the fixes and move on.
Then, run linting and formatting checks with `pixi run check` and
address any remaining issues until the code is clean.
This also regenerates `docs/architecture/package-structure-full.md`
and `docs/architecture/package-structure-short.md` automatically — do
not edit those files by hand. Do not check what was auto-fixed, just
accept the fixes and move on. Then, run linting and formatting checks
with `pixi run check` and address any remaining issues until the code
is clean.
- After changes, run unit tests with `pixi run unit-tests`.
- After changes, run integration tests with
`pixi run integration-tests`.
Expand Down
132 changes: 72 additions & 60 deletions docs/architecture/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,6 @@ experiment.data # CategoryCollection
experiment.background_type = 'chebyshev' # triggers BackgroundFactory.create(...)
experiment.peak_profile_type = 'thompson-cox-hastings' # triggers PeakFactory.create(...)
experiment.extinction_type = 'becker-coppens' # triggers ExtinctionFactory.create(...)
experiment.linked_crystal_type = 'default' # triggers LinkedCrystalFactory.create(...)
experiment.excluded_regions_type = 'default' # triggers ExcludedRegionsFactory.create(...)
experiment.linked_phases_type = 'default' # triggers LinkedPhasesFactory.create(...)
```

**Type switching pattern:** `expt.background_type = 'chebyshev'` rather
Expand Down Expand Up @@ -389,26 +386,26 @@ from .line_segment import LineSegmentBackground

### 5.5 All Factories

| Factory | Domain | Tags resolve to |
| ---------------------------- | ---------------------- | ------------------------------------------------------------ |
| `BackgroundFactory` | Background categories | `LineSegmentBackground`, `ChebyshevPolynomialBackground` |
| `PeakFactory` | Peak profiles | `CwlPseudoVoigt`, `TofJorgensen`, `TofJorgensenVonDreele`, … |
| `InstrumentFactory` | Instruments | `CwlPdInstrument`, `TofPdInstrument`, … |
| `DataFactory` | Data collections | `PdCwlData`, `PdTofData`, `ReflnData`, `TotalData` |
| `ExtinctionFactory` | Extinction models | `BeckerCoppensExtinction` |
| `LinkedCrystalFactory` | Linked-crystal refs | `LinkedCrystal` |
| `ExcludedRegionsFactory` | Excluded regions | `ExcludedRegions` |
| `LinkedPhasesFactory` | Linked phases | `LinkedPhases` |
| `ExperimentTypeFactory` | Experiment descriptors | `ExperimentType` |
| `CellFactory` | Unit cells | `Cell` |
| `SpaceGroupFactory` | Space groups | `SpaceGroup` |
| `AtomSitesFactory` | Atom sites | `AtomSites` |
| `AliasesFactory` | Parameter aliases | `Aliases` |
| `ConstraintsFactory` | Parameter constraints | `Constraints` |
| `FitModeFactory` | Fit-mode category | `FitMode` |
| `JointFitExperimentsFactory` | Joint-fit weights | `JointFitExperiments` |
| `CalculatorFactory` | Calculation engines | `CryspyCalculator`, `CrysfmlCalculator`, `PdffitCalculator` |
| `MinimizerFactory` | Minimisers | `LmfitMinimizer`, `DfolsMinimizer`, … |
| Factory | Domain | Tags resolve to |
| ---------------------------- | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `BackgroundFactory` | Background categories | `LineSegmentBackground`, `ChebyshevPolynomialBackground` |
| `PeakFactory` | Peak profiles | `CwlPseudoVoigt`, `TofJorgensen`, `TofJorgensenVonDreele`, … |
| `InstrumentFactory` | Instruments | `CwlPdInstrument`, `TofPdInstrument`, … |
| `DataFactory` | Data collections | `PdCwlData`, `PdTofData`, `ReflnData`, `TotalData` |
| `ExtinctionFactory` | Extinction models | `BeckerCoppensExtinction` |
| `LinkedCrystalFactory` | Linked-crystal refs | `LinkedCrystal` |
| `ExcludedRegionsFactory` | Excluded regions | `ExcludedRegions` |
| `LinkedPhasesFactory` | Linked phases | `LinkedPhases` |
| `ExperimentTypeFactory` | Experiment descriptors | `ExperimentType` |
| `CellFactory` | Unit cells | `Cell` |
| `SpaceGroupFactory` | Space groups | `SpaceGroup` |
| `AtomSitesFactory` | Atom sites | `AtomSites` |
| `AliasesFactory` | Parameter aliases | `Aliases` |
| `ConstraintsFactory` | Parameter constraints | `Constraints` |
| `FitModeFactory` | Fit-mode category | `FitMode` |
| `JointFitExperimentsFactory` | Joint-fit weights | `JointFitExperiments` |
| `CalculatorFactory` | Calculation engines | `CryspyCalculator`, `CrysfmlCalculator`, `PdffitCalculator` |
| `MinimizerFactory` | Minimisers | `LmfitMinimizer`, `LmfitLeastsqMinimizer`, `LmfitLeastSquaresMinimizer`, `DfolsMinimizer`, `BumpsMinimizer`, `BumpsLmMinimizer`, `BumpsAmoebaMinimizer`, `BumpsDEMinimizer` |

> **Note:** `ExperimentFactory` and `StructureFactory` are _builder_
> factories with `from_cif_path`, `from_cif_str`, `from_data_path`, and
Expand Down Expand Up @@ -509,16 +506,16 @@ Tags are the user-facing identifiers for selecting types. They must be:

**Minimizer tags**

| Tag | Class |
| ----------------------- | ----------------------------------------- |
| `lmfit` | `LmfitMinimizer` |
| `lmfit (leastsq)` | `LmfitMinimizer` (method=`leastsq`) |
| `lmfit (least_squares)` | `LmfitMinimizer` (method=`least_squares`) |
| `dfols` | `DfolsMinimizer` |

> **Note:** minimizer variant tags (`lmfit (leastsq)`,
> `lmfit (least_squares)`) are planned but not yet re-implemented after
> the `FactoryBase` migration. See `issues_open.md` for details.
| Tag | Class |
| ----------------------- | ---------------------------- |
| `lmfit` | `LmfitMinimizer` |
| `lmfit (leastsq)` | `LmfitLeastsqMinimizer` |
| `lmfit (least_squares)` | `LmfitLeastSquaresMinimizer` |
| `dfols` | `DfolsMinimizer` |
| `bumps` | `BumpsMinimizer` |
| `bumps (lm)` | `BumpsLmMinimizer` |
| `bumps (amoeba)` | `BumpsAmoebaMinimizer` |
| `bumps (de)` | `BumpsDEMinimizer` |

### 5.7 Metadata Classification — Which Classes Get What

Expand Down Expand Up @@ -605,6 +602,7 @@ line-segment points.
| `PdffitCalculator` | `CalculatorFactory` | (same) |
| `LmfitMinimizer` | `MinimizerFactory` | `type_info` only |
| `DfolsMinimizer` | `MinimizerFactory` | (same) |
| `BumpsMinimizer` | `MinimizerFactory` | (same) |
| `BraggPdExperiment` | `ExperimentFactory` | `type_info` + `compatibility` (no `calculator_support`) |
| `TotalPdExperiment` | `ExperimentFactory` | (same) |
| `CwlScExperiment` | `ExperimentFactory` | (same) |
Expand Down Expand Up @@ -636,7 +634,7 @@ The experiment exposes the standard switchable-category API:
### 6.2 Minimiser

The minimiser drives the optimisation loop. `MinimizerFactory` creates
instances by tag (e.g. `'lmfit'`, `'dfols'`).
instances by tag (e.g. `'lmfit'`, `'dfols'`, `'bumps'`).

### 6.3 Fitter

Expand All @@ -658,14 +656,19 @@ workflow:
- Fit mode: `fit_mode` (`CategoryItem` with a `mode` descriptor
validated by `FitModeEnum`); `'single'` fits each experiment
independently, `'joint'` fits all simultaneously with weights from
`joint_fit_experiments`.
`joint_fit_experiments`, `'sequential'` records that sequential
fitting was used. `show_supported_fit_mode_types()` filters by
experiment count (≤1 → only `single`; >1 → all three).
`show_current_fit_mode_type()` prints the current mode.
- Joint-fit weights: `joint_fit_experiments` (`CategoryCollection` of
per-experiment weight entries); sibling of `fit_mode`, not a child.
- Parameter tables: `show_all_params()`, `show_fittable_params()`,
`show_free_params()`, `how_to_access_parameters()`
- Fitting: `fit()`, `show_fit_results()`
- Aliases and constraints (switchable categories with `aliases_type`,
`constraints_type`, `fit_mode_type`, `joint_fit_experiments_type`)
- Fitting: `fit()` dispatches single/joint; `fit_sequential()` handles
sequential mode (sets `fit_mode` to `'sequential'` internally).
`display.fit_results()` shows results.
- Aliases and constraints (single-type categories; no public `_type`
getter or setter)

---

Expand Down Expand Up @@ -955,8 +958,11 @@ and simplifies maintenance.
Categories whose concrete implementation can be swapped at runtime
(background, peak profile, etc.) are called **switchable categories**.
**Every category must be factory-based** — even if only one
implementation exists today. This ensures a uniform API, consistent
discoverability, and makes adding a second implementation trivial.
implementation exists today. This ensures uniform construction,
consistent metadata, and makes adding a second implementation trivial.

For categories with **multiple implementations** (multi-type), the owner
exposes the full switchable API:

| Facet | Naming pattern | Example |
| --------------- | -------------------------------------------- | ------------------------------------------------ |
Expand All @@ -965,15 +971,33 @@ discoverability, and makes adding a second implementation trivial.
| Show supported | `show_supported_<category>_types()` | `expt.show_supported_background_types()` |
| Show current | `show_current_<category>_type()` | `expt.show_current_peak_profile_type()` |

The convention applies universally:
Multi-type categories:

- **Experiment:** `calculator_type`, `background_type`,
`peak_profile_type`, `extinction_type`, `linked_crystal_type`,
`excluded_regions_type`, `linked_phases_type`, `instrument_type`,
`data_type`.
- **Structure:** `cell_type`, `space_group_type`, `atom_sites_type`.
- **Analysis:** `aliases_type`, `constraints_type`, `fit_mode_type`,
`joint_fit_experiments_type`.
`peak_profile_type`, `extinction_type`.

Categories that are **fixed at creation** (determined by the experiment
type and never changed) expose only a read-only `<category>` property
with no `_type` getter, setter, or show methods:

- **Experiment:** `instrument`, `data`.

For categories with **only one implementation** (single-type), the
`_type` getter, setter, and show methods are omitted from the public API
to avoid clutter. The factory and `_type` attribute still exist
internally for consistency and future extensibility.

Single-type categories (no public `_type` property):

- **Experiment:** `diffrn`, `linked_crystal`, `excluded_regions`,
`linked_phases`.
- **Structure:** `cell`, `space_group`, `atom_sites`.
- **Analysis:** `aliases`, `constraints`.

`fit_mode` has show methods (`show_supported_fit_mode_types()`,
`show_current_fit_mode_type()`) but no public `_type` getter or setter
because it has only one factory implementation. The mode is changed via
the `fit_mode.mode` descriptor directly.

**Design decisions:**

Expand All @@ -998,18 +1022,6 @@ expt.show_supported_peak_profile_types()
expt.show_supported_background_types()
expt.show_supported_calculator_types()
expt.show_supported_extinction_types()
expt.show_supported_linked_crystal_types()
expt.show_supported_excluded_regions_types()
expt.show_supported_linked_phases_types()
expt.show_supported_instrument_types()
expt.show_supported_data_types()
struct.show_supported_cell_types()
struct.show_supported_space_group_types()
struct.show_supported_atom_sites_types()
project.analysis.show_supported_aliases_types()
project.analysis.show_supported_constraints_types()
project.analysis.show_supported_fit_mode_types()
project.analysis.show_supported_joint_fit_experiments_types()
project.analysis.show_available_minimizers()
```

Expand All @@ -1026,7 +1038,7 @@ class. This applies to:
- Factory tags (§5.6) — e.g. `PeakProfileTypeEnum`, `CalculatorEnum`.
- Experiment-axis values — e.g. `SampleFormEnum`, `BeamModeEnum`.
- Category descriptors with enumerated choices — e.g. fit mode
(`FitModeEnum.SINGLE`, `FitModeEnum.JOINT`).
(`FitModeEnum.SINGLE`, `FitModeEnum.JOINT`, `FitModeEnum.SEQUENTIAL`).

The enum serves as the **single source of truth** for valid values,
their user-facing string representations, and their descriptions.
Expand Down
34 changes: 34 additions & 0 deletions docs/architecture/issues_closed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ Issues that have been fully resolved. Kept for historical reference.

---

## Restore Minimiser Variant Support

Used thin subclasses (approach A) to restore lmfit algorithm variants.
`LmfitLeastsqMinimizer` and `LmfitLeastSquaresMinimizer` extend
`LmfitMinimizer`, each with its own `TypeInfo` tag. Added
`MinimizerTypeEnum` for all minimizer tags. No `FactoryBase` changes
needed — one class per tag.

---

## Implement `Project.load()`

`Project.load(dir_path)` classmethod reads `project.cif`,
Expand Down Expand Up @@ -89,3 +99,27 @@ output. Unified `plot_param_series()` to read from CSV. Added
also writes CSV. Prerequisites included: CIF truncation fix, CIF
round-trip verification, `analysis.cif` moved into `analysis/`
directory, `extract_data_paths_from_zip` destination parameter.

---

## Make `data_type` Read-Only on Experiments

Removed the `data_type` setter, `show_supported_data_types()`, and
`show_current_data_type()` from both `ScExperimentBase` and
`PdExperimentBase`. The data collection type is now fixed at experiment
creation (resolved from `DataFactory.default_tag(...)` using the
experiment's axes), like experiment type itself. This prevents switching
to an incompatible data class and silently discarding loaded data.

---

## 78. Add `SEQUENTIAL` to `FitModeEnum` and Show Methods to Analysis

Added `SEQUENTIAL = 'sequential'` to `FitModeEnum`. `fit_sequential()`
now sets `fit_mode.mode = 'sequential'` internally so the mode is
persisted in CIF. Added `show_supported_fit_mode_types()` (filters by
experiment count: ≤1 → only `single`; >1 → all three) and
`show_current_fit_mode_type()` on `Analysis`. If `fit()` is called while
mode is `'sequential'`, it logs an error directing the user to
`fit_sequential()`. Promoted `fit_mode` from a pure single-type category
to one with show methods.
Loading
Loading