Skip to content

Add a very basic runtime check setup#17

Open
tmadlener wants to merge 6 commits intomainfrom
add-tests
Open

Add a very basic runtime check setup#17
tmadlener wants to merge 6 commits intomainfrom
add-tests

Conversation

@tmadlener
Copy link
Copy Markdown
Member

BEGINRELEASENOTES

  • Add a very basic test setup that uses CaloClouds3 to verify that things run
  • "Fix" some reference counting issue by leaking a resource (with a proper fix deferred to the future)

ENDRELEASENOTES

The ref-counting issue was introduced with #10 (but since we didn't have any tests it wasn't notice then).

@tmadlener
Copy link
Copy Markdown
Member Author

I cannot reproduce the test failures that CI sees "locally" for some reason.

@tmadlener
Copy link
Copy Markdown
Member Author

Using a debug build this test seems to uncover a true issue that has laid dormant up until now. Specifically, this assert fires:

assert(inputs.size() == tensDims.size());

That in turn is becaus CaloCloudsTwoAngleModel indeed defines 4 dimensions in the tensor but only fills three inputs:

TensorDimVecs m_tensDims = {{1, 1}, {1, 1}, {1, 1}, {1, 3}};

// For now, assume batch size one, and just assign values
inputs[0][0] = energy / CLHEP::GeV; // E_vec[0]/100.;
inputs[1][0] = theta; // 89.*(M_PI/180.) ; //Theta_vec[0]/(90.*(M_PI/180.));
inputs[2][0] = phi;

Not sure which of these need to be fixed.

Avoids some refernce counting issues **but needs a proper fix**
Make them correspond to what we actually pass on to inference as this is
asserted
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.

Runtime test that can be run in CI

1 participant