Repository navigation
task: agent ready prep - #280
jharlow-intel wants to merge 3 commits into
Conversation
c80b57b to
823e526
Compare
- CONTRIBUTING.md: use --override-channels, install pre-commit with the pylint and shellcheck its system hooks need, note Windows support and the meson.build GCC/MSVC branches, and match the CHANGELOG link format - AGENTS.md: restore supported platforms, add the meson.build guardrail, and note that checkout tests need the editable install - conda-recipe-cf/AGENTS.md: gnu_thread is Linux-only, llvm-openmp has no run pin, and each recipe has its own conda_build_config.yaml - mkl_umath/src/AGENTS.md: scope the patch lock claim and note per-thread patch/restore balance - .github/AGENTS.md: list the templates and repo config files
napetrov
left a comment
There was a problem hiding this comment.
I checked the docs at 3f8573f against the tree and by running them.
- I ran the
CONTRIBUTING.mdsetup verbatim in a cleancondaforge/miniforge3container. Creating the env, the editable install,pytest mkl_umath/tests(392 passed, 1 skipped) andpre-commit run --all-filesall pass. A control build without-Dmkl_threading=gnu_threadfails at import withlibmkl_intel_thread.so.3: undefined symbol: __atomic_compare_exchange, so thegnu_threadnote is right. - I also ran a static check of the paths, commands and identifiers the guidance names. At head it finds no broken references. At base it found the stale
mkl_randommentions and thesrc/__umath_generated.cpath, which this PR fixes. - I checked loading with Claude Code 2.1.296. The root
AGENTS.mdloads from every directory, and each nested one loads once the agent works in its subtree..github/copilot-instructions.mdis never loaded; more on that inline.
A few remaining points are inline. None of them block the PR.
| On Linux: | ||
|
|
||
| ```sh | ||
| conda create -n mkl_umath-dev --override-channels -c conda-forge python pip \ |
There was a problem hiding this comment.
python is unpinned here. conda-forge already has Python 3.15.0, while pyproject.toml has requires-python = ">=3.10,<3.15". Today the solve picks 3.14.8 (checked), likely because not every dependency has a 3.15 build yet. Once they all do, this line creates a 3.15 env and the editable install fails on requires-python. I suggest "python<3.15", or a specific python=3.14.
| - Treat recipe files as canonical for packaging intent and dependency pins. | ||
| - Keep recipe changes in step with `.github/workflows/conda-package.yml`, which | ||
| builds this recipe and runs the full test suite against the result. | ||
| - The package test in `meta.yaml` runs only `test_basic.py`. |
There was a problem hiding this comment.
It's worth saying that CI never runs this package test. Both conda workflows call conda build --no-test (conda-package.yml:286, conda-package-cf.yml:275) and run pytest --pyargs mkl_umath in a separate job. Without that note, an agent can assume CI covers a change to the meta.yaml test.
| ## Development guardrails | ||
| - Preserve NumPy ufunc behavior; patched loops stand in for NumPy's own. | ||
| - Edit the `*.src` templates and `generate_umath.py`, not generated C. | ||
| - Keep the floating-point precision flags in `meson.build`; Intel-only flags stay |
There was a problem hiding this comment.
/fp:precise is also set for Windows in both recipes' conda_build_config.yaml (line 20, # [win]), not only in meson.build. Could this line, and CONTRIBUTING.md:47, say "in meson.build and the recipes' conda_build_config.yaml"? That way an agent editing one place knows there is a second.
| ## Contribution expectations | ||
| - Keep changes atomic and single-purpose. | ||
| - Preserve runtime patching API (`use_in_numpy()`, `restore()`, `is_patched()`) unless explicitly requested. | ||
| - Preserve the runtime patching API (`patch_numpy_umath()`, `restore_numpy_umath()`, |
There was a problem hiding this comment.
This rule exists only in copilot-instructions.md. Claude Code never loads that file (checked with 2.1.296 from every subdirectory), and agents other than Copilot see it only if they follow the pointer at AGENTS.md:53. If the rule should apply to all agents, copying this line into the root AGENTS.md guardrails would cover them.
Also, use_in_numpy() and restore() are still exported in __all__ and exercised in test_patching.py. If they should stay until the deprecation removal, list them here too, or say that they may be removed.
Very similar to IntelPython/mkl_fft#379: adds contributor and coding-agent guidance (
CONTRIBUTING.md,AGENTS.md), PR and issue templates, and a README pointer, checked against this repo's build and CI. It also corrects the existingAGENTS.mdfiles.