Commit 7c815cc6 authored by Vacaliuc, Bogdan's avatar Vacaliuc, Bogdan
Browse files

response to prompt 2 and prompt 2.1 after my discussion with Tim

parent 95320c7a
Loading
Loading
Loading
Loading
+40 −0
Original line number Diff line number Diff line
@@ -150,3 +150,43 @@ You are working on the tasking project, quicknxsv2-modularization branch. You ar

You are working on the tasking project, 'quicknxsv2-modularization-with-mantid-elaboration' branch. You are preparing documents to support a week-long software developer/neutron scientist "hack-a-thon". In a previous session you have performed a comprehensive analysis and have generated the documents in plan/quicknxsv2-modularization/*.md. You may read them to orient yourself. Following a discussion with the scientists a gap in understanding was identified in the Mantid algorithm mantid.simpleapi.MagnetismReflectometryReduction(). This algorithm was seen as a "black-box" that needed to be understood. The goal of this session is to *fully understand* the implementation of the Mantid algorithms that are called by quicknxs and mr_reduction. I have cloned the Mantid source code to the local folder `/media/ssd2/mantid/`. It contains the full git history and you may inspect it as you need. Please augment the knowlege base with your new understanding having inspected the Mantid source code and its use in quicknxs and mr_reduction. Be diligent and resourceful. If you need a tool to analyze the data that you do not have, use venv, uv and/or pixi to install it ask me to help you.

```
● Done. Summary of the session:

  New document:
  - plan/quicknxsv2-modularization/10-mantid-algorithms-deep-dive.md (848 lines) — a source-grounded walkthrough of MagnetismReflectometryReduction and its supporting Mantid algorithms.

  Amended documents: 03-mantid-and-mr_reduction-coupling.md, 00-executive-summary.md, 08-risks-and-open-questions.md, README.md — all now link to the deep-dive and carry the corrected findings.

  Three commits on branch quicknxsv2-modularization-with-mantid-elaboration:
  1. 2ee37f5 — adds the deep-dive
  2. 95320c7 — back-links the existing docs
  3. 2a21ac0 (parent uvdl3) — bumps the tasking gitlink

  Key findings surfaced from reading the Mantid source at /media/ssd2/mantid:

  1. MRR is 859 LOC of Python, not C++ — the existing 03 doc was wrong on this. It's a PythonAlgorithm registered via AlgorithmFactory.subscribe. The hackathon can read, patch, and debug it natively; no Mantid build required. Its only C++ dependency
  is RefRoi (~220 LOC).
  2. MR Python algorithms are byte-identical between Mantid 6.14 and HEAD — git log v6.14.0..HEAD on all four MR files (MagnetismReflectometryReduction, MRFilterCrossSections, MRInspectData, MRGetTheta) returns zero commits. The 6.14/6.15 pin
  mismatch is a packaging decision, not a science decision.
  3. Four MRR defaults where the consumers silently disagree: ErrorWeightedBackground, CropFirstAndLastPoints, RoundUpPixel, AcceptNullReflectivity — plus UseSANGLE (DANGLE vs SANGLE for every θ) and the post-hoc QuickNXS scaling factor that is
  always-applied in quicknxsv2 but only conditionally-applied in mr_reduction. Same file, two consumers, different R(Q).
  4. A canonical DataInfo lives inside Mantid itself as MRInspectData.DataInfo (474 LOC). Both mr_reduction.data_info.DataInfo (1101 LOC) and quicknxsv2.interfaces.data_handling.data_info.DataInfo (413 LOC) are parallel reimplementations. The
  consolidation could potentially replace both with sample-log-reading shims — a ~1000-LOC deletion opportunity.
  5. SingleReadoutDeadTimeCorrection is registered by three packages simultaneously (mr_reduction, quicknxsv2, lr_reduction), last-import-wins. It is not part of Mantid core.
```

### Prompt 2.1

Very good. Please assess the following points:

Re. 1: Are you able to inspect through to the RefRoi code to understand what it is doing? If so, would you update the findings?
Re. 3: Please elaborate on the disagreement/tension. This is the sort of thing that the hack-a-thon needs to uncover. Also, please define 'consumers' (and I apologize if you have defined it - my understanding is that it is the functions that use the objects that are produced by MRR(); do correct me if that is wrong).
Re. 4: This is part of the technical debt.
Re. 5: This is part of the technical debt.

Please review the quicknxsv2 again and add a new section summarizing the 'Technical Debt' that exists in the quicknxsv2 (and mr_reduction that it calls) code base. Be excruciatingly thorough. This question keeps coming up in discussions with the scientists and its a critical aspect of the effort. If you need clarification on any point of what 'Technical Debt' might be, please ask me.

Please also review the quicknxsv2 specifically looking for parameters and configuration settings and how they propagate. I know you covered this somewhat in '02-frontend-backend-boundary.md', but what I'm asking for is another deep dive. The reason for this is that discussions with scientists reveal concern that Qt data structures are referenced in parameter extractions in 'mr_reduction' functions and/or the wrappers and that these references are responsible for many of the problems observed. Please review the git log of quicknxsv2 thoroughly to look for "fixes" in which such propagation was the at the root cause of defects and improper operation.

If there are any updates to the documentation set in 'plan/quicknxsv2-modularization/' necessary as a result of the above, please do the updates.