Skip to content

Updating the main branch in Daniel's root repo with my changes after completing 2606.30729 - #3

Open
SeppeStaelens wants to merge 17 commits into
drmayerson:ConfigReaderfrom
SeppeStaelens:from_config_reader
Open

Updating the main branch in Daniel's root repo with my changes after completing 2606.30729#3
SeppeStaelens wants to merge 17 commits into
drmayerson:ConfigReaderfrom
SeppeStaelens:from_config_reader

Conversation

@SeppeStaelens

Copy link
Copy Markdown
Contributor

Lengthy PR to start reviewing changes made to the FOORT code after finishing 2606.30729. On the top level, this includes

  • the addition of the rotating boson star metric
  • better handling of the ISCO finder
  • additions to postprocessing.

This PR will likely stay open for a while, to review the changes in some more detail. After review, a large codebase review will be carried out with the help of some AI tool, to catch any final issues.

@SeppeStaelens
SeppeStaelens marked this pull request as ready for review July 16, 2026 13:55
@SeppeStaelens

Copy link
Copy Markdown
Contributor Author

Starting review. Not ready to merge yet.

@SeppeStaelens SeppeStaelens left a comment

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.

Mostly comments to myself, to clean and fix before it can sensibly be pushed to the main repo.

Comment thread FOORT/src/Config.cpp
AllDiagSettings["EquatorialEmission"].LookupValue("ISCOLowerBound", iscolowerbound);
AllDiagSettings["EquatorialEmission"].LookupValue("ISCOUpperBound", iscoupperbound);
}
else

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.

The fallback logic needs to be checked here: at the moment it always falls back to GeneralCircularRadial

real upperbound{1000.0};
real lowerbound{m_ISCOlowerbound};
real upperbound{m_ISCOupperbound};
const SphericalHorizonMetric *sphermetric = dynamic_cast<const SphericalHorizonMetric *>(m_theMetric);

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.

This is perhaps something I need to look at as well: since I now made ISCO lower and upper bound configurable, it is perhaps not logical that these parameters will get overwritten if the metric is a SphericalHorizonMetric

Comment thread FOORT/src/Metric.h
BicubicSplineInterpolator *m_lInterpolator;
BicubicSplineInterpolator *m_gInterpolator;
BicubicSplineInterpolator *m_OmegaInterpolator;

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.

The parameter L is actually outdated: should maybe get rid of it first

Comment thread FOORT/test/CMakeLists.txt
@@ -1,12 +1,24 @@
add_executable(test_integrator test_integrator.cpp)
add_executable(test_einstein_ring test_einstein_ring.cpp)
add_executable(test_rotating_boson_star_geodesic test_rotating_boson_star_geodesic.cpp)

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.

I probably need to remove the rotating BS tests if the files are not publicly available

Comment thread .gitignore
##
## Get latest from https://github.com/github/gitignore/blob/master/VisualStudio.gitignore

# Additional

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.

Move these outside of .gitignore, as these are all currently in the repo. Either move to exclude or prune

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.

1 participant