Skip to content

Metric to Development Pull Request - #134

Open
DK-ombudsman wants to merge 65 commits into
developmentfrom
metric
Open

DK-ombudsman wants to merge 65 commits into
developmentfrom
metric

Conversation

@DK-ombudsman

Copy link
Copy Markdown
Collaborator

No description provided.

DK-ombudsman and others added 30 commits June 5, 2026 17:19
…d of interpolate_rhs_from_mesh to use Metric.H
@@ -0,0 +1,86 @@
'''

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for constant backgrounds, there is an option in emu to specify the rho/T/Ye in the parameter file without creating a background file. That might be the more simple way to do it to minimize the amount of code that needs to be maintained.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Scripts/util/tab_exterminator.sh Outdated
@@ -1,5 +1,5 @@
#!/bin/bash
for f in $(grep -rIPl "\t" ../../Source)
for f in $(grep -rIl $'\t' ../../Source)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why was this changed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That was committed by mistake. That's a change which is particular to my macOS system

Comment thread Source/Evolve.cpp Outdated
if (parms->cfl_factor > 0.0) {
Real min_length;

if (parms->coord_sys == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rather than having an if statement here, it should just ask the metric about the min length. That way the only if statement is the one that chooses the metric.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Source/Evolve.cpp Outdated
p.rdata(PIdx::pupz) / p.rdata(PIdx::pupt)};

// For curvilinear coordinates, we convert phat to curvilinear components projected on a local orthonormal tetrad for each particle
if (parms->coord_sys != 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rather than having if statements here, this should just be a function contained in the metric. That way the only if statement is the one that chooses the metric.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the cartesian metric can trivially just return without doing anything

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Source/FlavoredNeutrinoContainer.cpp Outdated

FourVec pos_int;

if (coord_sys == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same statement regarding if statements - this logic should be in the metric class.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Source/FlavoredNeutrinoContainer.cpp Outdated
mode == BoundaryCondition::outflow) {
p.pos(d) = reflected_pos;

if (parms->coord_sys == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same statement regarding if statements - this logic should be in the metric class.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Source/FlavoredNeutrinoContainer.cpp Outdated

p.rdata(PIdx::pupx + d) = -p.rdata(PIdx::pupx + d);

if (parms->coord_sys == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same statement regarding if statements - this logic should be in the metric class.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done


//calculating cell volume
amrex::Real V_cell;
if (coord_sys == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same statement regarding if statements - this logic should be in the metric class.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread Source/Metric.H
@DK-ombudsman DK-ombudsman reopened this Aug 31, 2026
@cellio00

cellio00 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

I have not yet weighed in on the PR until Debraj/Sherwood finish their discussions from Sherwood's review. Just as an update!! I have seen it, just waiting for my turn.

@srichers srichers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall, looks good. Only a couple of small modifications to apply.

Comment thread Source/FlavoredNeutrinoContainer.cpp Outdated

metric.coord_conv_inv(p);

// p.rdata(PIdx::pupx) = p.rdata(PIdx::pupx) + 0.05*p.rdata(PIdx::pupt);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If this is not used, should we delete the code?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. deleted.

Comment thread Source/Evolve.cpp Outdated
for (int j = sy.first(); j <= sy.last(); ++j) {
for (int i = sx.first(); i <= sx.last(); ++i) {
// getting the upper and lower bounds of the cell
const amrex::Real x1_lo = plo[0] + i / dxi[0];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this code is repeated below, so I think we should turn it into a function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It does not have any coordinate system components to it ( so it's same for all coordinate systems). Should the function still be in metric or where should I put the function?

Comment thread Source/Metric.H Outdated

@cellio00 cellio00 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I approve of these changes; however It looks like there are requested changes still.

@cellio00
cellio00 requested a review from srichers September 17, 2026 21:33
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.

3 participants