Fix PhotulatorModelStack.fluxes; guard against legacy n_layers-1 artifacts#3
Open
ixkael wants to merge 2 commits into
Open
Fix PhotulatorModelStack.fluxes; guard against legacy n_layers-1 artifacts#3ixkael wants to merge 2 commits into
ixkael wants to merge 2 commits into
Conversation
…thod
PhotulatorModelStack.fluxes() called self.emulators[i].fluxes(theta, N),
but neither Photulator nor PhotulatorBasic define a fluxes() method — only
the singular flux(theta, N), which returns flux in maggies. Any caller
using the stack's fluxes() path hit an AttributeError.
Usage (doctest-style, no test/ layout exists on this branch to add a unit
test to):
>>> stack = PhotulatorModelStack(root_dir="trained_models/", filenames=[...])
>>> stack.fluxes(theta, N).shape # (n_objects, n_filters), maggies
torch.Size([n_objects, n_filters])
Before this fix, the above raised:
AttributeError: 'Photulator' object has no attribute 'fluxes'
Two incompatible generations of trained Photulator artifact exist. Current artifacts were trained with the parametrized activation applied to every layer including the output projection, and carry n_layers alpha/beta pairs, matching Photulator.forward()'s zip(self.W, self.b, self.alphas, self.betas) loop. Older, TF-converted artifacts trained a linear output layer and carry only n_layers-1 alpha/beta pairs -- no activation was ever trained or saved for the output layer. Loading such a legacy artifact through PhotulatorModelStack previously produced no error: forward()'s zip loop silently truncates to the shortest input, either dropping the final linear projection from the computation, or (depending on how the artifact was constructed) pairing it with uninitialized, randomly-drawn activation parameters that are never overwritten. Either way, output magnitudes are wrong with no warning; on a real (non-mock) artifact this has produced ~48.6 mag of disagreement against the correct value, with 2.3e-5 agreement recovered once forward-pass on that artifact is redone with a linear (unactivated) final layer. Add _check_photulator_layer_parameters(), called from PhotulatorModelStack.__init__() right after each torch.load(), which compares the alpha/beta arrays an artifact actually supplies (as loaded) against its own n_layers and raises a ValueError naming the file and pointing to a linear-output forward pass as the correct evaluation path for such artifacts. PhotulatorBasic (no alphas/betas; fixed activation) is left untouched by the check. Manually verified: a toy Photulator with alphas/betas truncated to n_layers-1 (simulating a legacy artifact) now raises ValueError at PhotulatorModelStack load time instead of silently loading; a normal n_layers-pair artifact is unaffected and stack.fluxes() still matches per-emulator .flux().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
1.
PhotulatorModelStack.fluxes()called a nonexistent per-emulator method.PhotulatorModelStack.fluxes()calledself.emulators[i].fluxes(theta, N),but neither
PhotulatornorPhotulatorBasicdefine afluxes()method —only the singular
flux(theta, N)(returns flux in maggies). Any callerusing the stack's
fluxes()path hit:One-line fix: call
.flux()instead of.fluxes(). Notests/layoutexists on this branch, so the commit message includes a doctest-style usage
note instead of a unit test.
2. Guard against loading legacy
n_layers-1Photulator artifacts.Two incompatible generations of trained
Photulatorartifact exist:activation applied to every layer, including the output projection, and
carry
n_layersalpha/beta pairs — matchingPhotulator.forward()'szip(self.W, self.b, self.alphas, self.betas)loop.carry only
n_layers - 1alpha/beta pairs — no activation was evertrained (or saved) for the output layer.
Loading the latter through
PhotulatorModelStackpreviously produced noerror.
forward()'s zip loop silently truncates to the shortest input:depending on how the artifact was constructed, this either drops the final
linear projection from the computation entirely, or pairs it with
uninitialized, randomly-drawn activation parameters that are never
overwritten. Either way, output magnitudes are wrong with no warning. On a
real (non-mock) artifact this produced ~48.6 mag of disagreement against
the correct value, with 2.3e-5 agreement recovered once the forward pass
for that artifact was redone with a linear (unactivated) final layer — the
artifact itself was fine, only the forward convention applied to it was
wrong.
This PR adds
_check_photulator_layer_parameters(), called fromPhotulatorModelStack.__init__()right after eachtorch.load(). Itcompares the alpha/beta arrays an artifact actually supplies (as loaded)
against its own
n_layers, and raises aValueErrornaming the file andpointing to a linear-output forward pass (
out = out @ W[-1] + b[-1], noactivation on the final layer) as the correct evaluation path for such
artifacts, instead of silently loading.
PhotulatorBasic(noalphas/betas; fixed, untrainable activation) is left untouched by thecheck.
Testing
No
tests/directory exists on this branch. Manually verified:Photulator, saved/loaded throughPhotulatorModelStack, hasstack.fluxes()succeed and matchtorch.cat([e.flux(theta, N) for e in stack.emulators]).Photulatorwithalphas/betastruncated ton_layers - 1(simulating a legacy artifact) now raises
ValueErroratPhotulatorModelStackload time instead of silently loading.n_layers-pair artifact is unaffected by the new check.🤖 Generated with Claude Code