Architecture parsers are dropped when Core is linked as a static library
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 55/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- cmake, cpp
- Domain
- build-system
Research direction
Start with get_dsp.cpp and the parser translation units named in the report: container.cpp, convnet.cpp, linear.cpp, lstm.cpp, sequential.cpp, and wavenet/model.cpp. Reproduce the direct-object and libnam.a link commands on a supported platform, then trace which parser registration symbols are retained. Done means the chosen fix makes a static-library consumer load the A2 model with its parser registered, without relying on undocumented platform-specific linker flags.
Written by the indexing model from the issue text.
Description
Summary
When Core is compiled into a static library and linked into a consumer, every
architecture parser silently disappears. Loading any model then fails with:
No config parser registered for architecture: SlimmableContainer
The parsers are registered from static initializers, one per translation unit
(container.cpp, convnet.cpp, linear.cpp, lstm.cpp, sequential.cpp,
wavenet/model.cpp). A linker only pulls an archive member in when something
already in the link references one of its symbols. Nothing references those TUs
— the registration is the whole point — so they are dropped, and their
initializers never run.
Upstream's own CMake never hits this: tools/CMakeLists.txt passes the sources
directly to add_executable, so every object is always in the link. The bug only
shows up for consumers who build Core as a library, which is the normal way to
embed it.
Reproduction
Same objects, two link strategies. Only the second one fails.
git clone --recurse-submodules https://github.com/sdatkinson/NeuralAmpModelerCore.git
cd NeuralAmpModelerCore
cat > /tmp/main.cpp <<'CPP'
#include <iostream>
#include "NAM/get_dsp.h"
int main(int argc, char** argv) {
try {
auto dsp = nam::get_dsp(std::filesystem::path(argv[1]));
std::cout << "loaded OK, expected sample rate = " << dsp->GetExpectedSampleRate() << "\n";
return 0;
} catch (const std::exception& e) {
std::cout << "FAILED: " << e.what() << "\n";
return 1;
}
}
CPP
INC="-I. -IDependencies/eigen -IDependencies/nlohmann"
FLAGS="-std=c++20 -O2 -DNAM_ENABLE_A2_FAST"
mkdir -p obj
for f in NAM/*.cpp NAM/wavenet/*.cpp; do
c++ $FLAGS $INC -c "$f" -o "obj/$(basename "${f%.cpp}").o"
done
# A) objects linked directly — this is what upstream's CMake does
c++ $FLAGS $INC /tmp/main.cpp obj/*.o -o direct
./direct example_models/A2.nam
# B) exact same objects, but through an archive
ar rcs libnam.a obj/*.o
c++ $FLAGS $INC /tmp/main.cpp libnam.a -o static
./static example_models/A2.nam
Observed on macOS 15 with AppleClang:
── A) sources linked directly (what upstream's CMake does) ──
loaded OK, expected sample rate = 48000
── B) same objects, but via a static library ──
FAILED: No config parser registered for architecture: SlimmableContainer
Reproduced on v0.5.4. The registration mechanism is unchanged on main
(2563c0f), which adds a sixth registrar in sequential.cpp.
Workaround for consumers
Force the archive members in. This is what we ended up doing, because it is the
only form that is identical on the three desktop platforms — --whole-archive,
-force_load and /WHOLEARCHIVE all spell it differently:
using ConfigParser = std::unique_ptr<nam::ModelConfig> (*)(const nlohmann::json&, double);
volatile ConfigParser g_anchor = nullptr;
void anchor_parsers() {
g_anchor = &nam::container::create_config;
g_anchor = &nam::convnet::create_config;
g_anchor = &nam::linear::create_config;
g_anchor = &nam::lstm::create_config;
g_anchor = &nam::wavenet::create_config;
}
Note that a const array of the same pointers does not work: the compiler
discards it as unused before the linker ever sees the references. The writes have
to be volatile.
Suggested fix
Any of these would remove the trap; the second seems cheapest:
- Export a
nam::register_builtin_parsers()that consumers call once, and
document it. - Reference the
create_configfunctions fromget_dsp.cpp. That TU is always
linked, since it defines the entry point everyone calls, so it drags the rest
in without any consumer-visible API change. - Document the
--whole-archive/-force_load//WHOLEARCHIVErequirement in
the README.
The current failure mode is quiet and points in the wrong direction: the error
says the architecture is not registered, which reads like an unsupported model
rather than a link-time problem.
- Dominant language
- C++
- Stars
- 931
- Forks
- 178
- Avg merge
- 3h 31m
- Merged PRs (30d)
- 7
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from sdatkinson/NeuralAmpModelerCore
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
-
documentation
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
sdatkinson/NeuralAmpModelerCore#305 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
sdatkinson/NeuralAmpModelerCore#304 · 2 comments ·
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
sdatkinson/NeuralAmpModelerCore#303 · 2 comments · 1 reaction ·
-
breaking
Difficulty 5/5 Over a week Newbie friendliness 35/100
sdatkinson/NeuralAmpModelerCore#294 · 1 reaction ·
All issues in sdatkinson/NeuralAmpModelerCore
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
-
Sensor initialization takes very long when `--initial-sim-time` is set to current UNIX timestamp Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
gazebosim/gz-sensors#662 · 1 comment ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
comp-datalake
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
ClickHouse/ClickHouse#121222 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
LadybirdBrowser/ladybird#12123 ·