Skip to content

Add unit tests for normalisation - #3476

Open
dschwoerer wants to merge 6 commits into
nextfrom
normalisation-unit-tests
Open

Add unit tests for normalisation#3476
dschwoerer wants to merge 6 commits into
nextfrom
normalisation-unit-tests

Conversation

@dschwoerer

Copy link
Copy Markdown
Contributor

No description provided.

@dschwoerer dschwoerer changed the title unit tests Add unit tests for normalisation Aug 27, 2026
Comment thread tests/unit/mesh/test_tokamak_coordinates.cxx Outdated
Comment thread src/mesh/coordinates.cxx Outdated
Comment on lines +1051 to +1053
if (jacobian_cache != nullptr) {
if (not jacobian_cache->isFci()) {
jacobian_cache.reset();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please could you add a comment explaining why we don't want to reset the jacobian in this case? I guess this is something to do with not wanting to recalculate the slices? But if we're calling this, then we've changed the metric, so how do we ensure the jacobian is in sync with the metric?

Also we can collapse the if statements into one :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please could you add a comment explaining why we don't want to reset the jacobian in this case? I guess this is something to do with not wanting to recalculate the slices?

Exactly. We could do this, but it is rather bothersome.

But if we're calling this, then we've changed the metric, so how do we ensure the jacobian is in sync with the metric?

We hope the user knows what they are doing. We also have no check that co and contra variant tensor are aligned. We could add a CHECK > 2 check to ensure J is somewhat consistent (and maybe co and contravariant tensors, too)

J needs to be preserved to ensure that parallel slices are preserved.
That can cause to NaNs for tokamaks and deletes parallel slices for FCI
@ZedThree

Copy link
Copy Markdown
Member

Oh, is the actual problem just the ordering of calls?

setJ(J);
setBxy(Bxy);
invalidateMetricCaches();

if (norm.J.has_value()) {
if (J().hasParallelSlices()) {
setJ(FieldMetricParallel{J() / *norm.J});
} else {
setJ(J() / *norm.J);
}
}
if (norm.Bxy.has_value()) {
if (Bxy().hasParallelSlices()) {
setBxy(FieldMetricParallel{Bxy() / *norm.Bxy});
} else {
setBxy(Bxy() / *norm.Bxy);
}
}
if (norm.dx.has_value()) {
setDx(dx() / *norm.dx);
}
if (norm.dy.has_value()) {
setDy(dy() / *norm.dy);
}
if (norm.dz.has_value()) {
setDz(dz() / *norm.dz);
}
invalidateMetricCaches();

Should we just move the call to invalidateMetricCaches() to before calling setJ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants