Skip to content

Conversation

@h-vetinari
Copy link
Member

Redo #114, now that MKL has miraculously been unblocked

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Oct 29, 2025

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ You're setting a constraint on the __osx virtual package directly; this should now be done by adding a build dependence on {{ stdlib("c") }}, and overriding c_stdlib_version in recipe/conda_build_config.yaml for the respective platform as necessary. For further details, please see META: {{ stdlib("c") }} migration conda-forge.github.io#2102.

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/19007357804. Examine the logs at this URL for more detail.

:: clang.exe cannot handle /LIBPATH: in LDFLAGS, but we need that for lld-link
set "CC=clang-cl.exe"

python %RECIPE_DIR%\create_forwarder_dll.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We filter the symbols in this file whereas create-forwarder-dll doesn't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean this part, right?

# A crude way to filter out unwanted symbols like fprintf which should
# not have been exported in netlib libraries. probably a flang bug
if symbol.startswith(("c", "s", "L", "d", "z", "i", "l", "x", "C", "R")):
symbols_lib_pairs.append((symbol, libname))

Would you be open to add this as an option in https://github.com/isuruf/create-forwarder-dll?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that part. I'm open to adding it. Maybe as a symbol filter regex?

@isuruf
Copy link
Member

isuruf commented Oct 29, 2025

@carterbox can you run NVPL tests?

@h-vetinari
Copy link
Member Author

Not very surprisingly (IMO), newaccelerate fails the 3.11 tests; AFAIU it only advertises compatibility with lapack 3.9. OTOH, the failures don't look catastrophically bad:

94% tests passed, 6 tests failed out of 93

Total Test time (real) =  41.32 sec

The following tests FAILED:
	  7 - LAPACK-xeigtsts_sec_in (Failed)
	 28 - LAPACK-xeigtstd_dec_in (Failed)
	 44 - LAPACK-xlintstc_ctest_in (Failed)
	 50 - LAPACK-xeigtstc_cec_in (Failed)
	 66 - LAPACK-xlintstz_ztest_in (Failed)
	 72 - LAPACK-xeigtstz_zec_in (Failed)

			-->   LAPACK TESTING SUMMARY  <--
		Processing LAPACK Testing output found in the TESTING directory
SUMMARY             	nb test run 	numerical error   	other error  
================   	===========	=================	================  
REAL             	385456		2	(0.001%)	3010	(0.781%)	
DOUBLE PRECISION	404304		0	(0.000%)	3032	(0.750%)	
COMPLEX          	345386		6032	(1.746%)	6745	(1.953%)	
COMPLEX16         	268973		7479	(2.781%)	6820	(2.536%)	

--> ALL PRECISIONS	1404119		13513	(0.962%)	19607	(1.396%)	

However, if we do not want to simply skip those xeigtst[sdcz] resp. xlintst[cz] tests, then I think we should probably allow the maximum lapack version to become independent per flavour; though we'd likely have to keep the compile-time pinning on 3.9 pretty much indefinitely then (for newaccelerate to remain something that can be switched in at runtime), which isn't very appealing TBH, but then I also don't see a viable alternative if we want to keep supporting this.

@carterbox
Copy link
Member

I built 2569478 natively for both NVPL and openBLAS. The tests pass.

@h-vetinari
Copy link
Member Author

@isuruf, I skipped the offending tests on newaccelerate (67bc0b7), which I think is the easier way forward for now, in terms of keeping a uniform baseline. PTAL

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.

4 participants