search

SWIFTSIM / pipeline-configs

Configuration files for the `pipeline`, each for different simulation sub-grid models.
1 stars 9 forks source link

Changes required to run on COLIBRE SOAP catalogues #233

Closed bwvdnbro closed 1 year ago

bwvdnbro commented 1 year ago

This PR includes the changes to the pipeline that are necessary to run on a SOAP catalogue instead of a VR catalogue. All changes affect the registration functions, where you have to use the general catalogue.get_quantity() function rather than attributes of the catalogue to access variables. Everything else (i.e. the autoplotter) works as before.

This PR also includes some fixes for warnings that triggered, mostly because of division by zero.

This PR requires https://github.com/SWIFTSIM/velociraptor-python/pull/95, since it uses the new VR catalogue interface introduced there.

MatthieuSchaller commented 1 year ago

@EvgeniiChaikin are we now in a position where we need this?

MatthieuSchaller commented 1 year ago

@EvgeniiChaikin I am happy for this to be merged. All changes make sense. I'll leave one last round of review to you.

EvgeniiChaikin commented 1 year ago

Sorry for the delay, I have been experiencing problems with running SOAP this weekend, so I couldn't properly test it.

MatthieuSchaller commented 1 year ago

That should be fine to merge, no? It does not break functionalities as far as I can tell.

EvgeniiChaikin commented 1 year ago

Indeed, everything should be working, as long as this branch is merged together with the other two branches in the SOAP (https://github.com/SWIFTSIM/SOAP/pull/12) and velociraptor-python (https://github.com/SWIFTSIM/velociraptor-python/pull/95) repository.

MatthieuSchaller commented 1 year ago

Yes, so I can merge this and people not using SOAP can still run. As long as I also merge the changes in vr-python.

EvgeniiChaikin commented 1 year ago

@MatthieuSchaller is there anything else that you would like to see in this PR?

Since the SOAP changes have been merged yesterday in velociraptor-python, but this PR is still hanging, the pipeline is currently broken.