Skip to content

Conversation

@vjcitn
Copy link

@vjcitn vjcitn commented Dec 12, 2023

reduce conditioning on USE_VBZ, helps with mac arm, will it work elsewhere? seems to work on linux. checking the action on my fork

reduce conditioning on USE_VBZ, helps with mac arm, will it work elsewhere?
@grimbough
Copy link
Collaborator

Thanks Vince. I've actually added some print statements to the configure script to try and understand why the USE_VBZ isn't being set. However the combination of having to wait a long time to get any feedback from the Mac ARM builder and that I'd need to make these diagnostic changes to the release branch means I've not actually committed them yet.

Perhaps the simplest, short-term, solution to at least get it building on Mac is to turn of the test for the VBZ filter in the case that the filter is missing. I have a function rhdf5filters::available_filters() which should detect which filters have been successfully compiled and let me selectively test only those.

Obviously it'd be nice to get this working properly, but I don't think the VBZ filter is particularly widely used. It's bespoke to Oxford Nanopore, and I don't get the impression many users will miss it.

@vjcitn
Copy link
Author

vjcitn commented Dec 12, 2023 via email

@grimbough
Copy link
Collaborator

Yes, I saw your messages in Slack. Thanks very much for looking in to possible solutions. It's indeed tricky to understand the fine details of this compilation issues without access to a Mac.

I think this solution is appropriate here since some systems can't actually run the VBZ code - CPUs older than about 2006 don't have the right instruction set - so it's valid not to provide it under those circumstances. That's what the USE_VBZ conditioning was trying to catch. However that's clearly not the case for modern Mac chips, so I need to figure out why my configure script is rejecting them.

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