Moving to src layout

[prady0t]

Description

Related to #4205

Fixes # (issue)

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[agriyakhetarpal]

Thanks, @prady0t! Could you trigger the wheel builds from this branch on your fork and link the workflow run here? I shall merge this in 24 hours unless someone raises any objections. Note that if any other PR gets merged before this one does, there might be conflicts again, so it will be better to resolve them here (or learn how to).

[Saransh-cpp]

Thanks, @prady0t! Looks good to me. I am not sure why the tests are failing but I'll try taking a look soon.

[agriyakhetarpal]

The tests are failing because

pybamm/solvers/c_solvers/idaklu/Expressions/Casadi/CasadiFunctions.cpp

is not found. To fix this, these lines have to be changed, which were missed:

PyBaMM/CMakeLists.txt

Lines 46 to 54 in 8983f43

	# Casadi PyBaMM source files
	set(IDAKLU_EXPR_CASADI_SOURCE_FILES "")
	if(${PYBAMM_IDAKLU_EXPR_CASADI} STREQUAL "ON" )
	add_compile_definitions(CASADI_ENABLE)
	set(IDAKLU_EXPR_CASADI_SOURCE_FILES
	pybamm/solvers/c_solvers/idaklu/Expressions/Casadi/CasadiFunctions.cpp
	pybamm/solvers/c_solvers/idaklu/Expressions/Casadi/CasadiFunctions.hpp
	)
	endif()

[agriyakhetarpal]

Note: the paths in scripts/update_version.py will likely also be broken. Let's ensure we test this thoroughly throughout the codebase from start to end, identify potential breakages, and fix them here.

[agriyakhetarpal]

A summary of the required changes:

1. ✅ Path to pybamm.root_dir()
2. ✅ Changes in scripts/update_version.py, and its corresponding update_version.yml file as needed
3. ✅ Paths to CITATIONS.bib in pyproject.toml and elsewhere
4. ✅ Changes in [tool.setuptools.package-data]
5. ✅ Changes to paths in conf.py (bibtex_bibfiles)
6. ✅ citations.py
7. ✅ Paths to version.py
8. ✅ Changes in CONTRIBUTING.md
9. ✅ A link to passing wheel builds triggered from this branch on your fork

There is a chance I might have missed something, so please feel free to suggest more.

[prady0t]

@agriyakhetarpal I've tried adding all the changes. Here's the workflow for wheels build : https://github.com/prady0t/PyBaMM/actions/runs/10228436876
Lets see how it goes

[prady0t]

Tests are still failing on the local branch so they will fail here too. Let me look into it further.

[agriyakhetarpal]

Thanks, nice to see wheels passing – besides fixing the CI failures, don't forget the rest of the points!

[agriyakhetarpal]

Let's focus on getting this PR merged before any of your other PRs. I posted about this PR in the #maintainers channel on Slack, and there seem to be no objections, so we will be good to go once the changes are addressed.

[prady0t]

prady0t#4
local branch tests are passing except for ubuntu

[agriyakhetarpal]

Keep going, this is almost there! To fix the failing builds, these paths need to be fixed:

PyBaMM/CMakeLists.txt

Lines 62 to 75 in 109b9c4

	# IREE (MLIR expression evaluation) PyBaMM source files
	set(IDAKLU_EXPR_IREE_SOURCE_FILES "")
	if(${PYBAMM_IDAKLU_EXPR_IREE} STREQUAL "ON" )
	add_compile_definitions(IREE_ENABLE)
	# Source file list
	set(IDAKLU_EXPR_IREE_SOURCE_FILES
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/iree_jit.cpp
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/iree_jit.hpp
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/IREEFunctions.cpp
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/IREEFunctions.hpp
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/ModuleParser.cpp
	pybamm/solvers/c_solvers/idaklu/Expressions/IREE/ModuleParser.hpp
	)
	endif()

Also, things like scripts/update_version.py and pybamm.root_dir(), etc., from the previous points are left to be updated; I checked off the items that have been addressed in #4311 (review).

[agriyakhetarpal]

Triggered it from the admin dashboard, let's see if it fails again

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.46%. Comparing base (fa68ddc) to head (f175d16).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4311   +/-   ##
========================================
  Coverage    99.46%   99.46%           
========================================
  Files          289      289           
  Lines        22146    22146           
========================================
  Hits         22027    22027           
  Misses         119      119           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[agriyakhetarpal]

I think the SPM notebook's failure is related to os.chdir(pybamm.__path__[0] + "/..") in the first cell. The coverage failure is unrelated. I don't understand the RTD failure right now. I think point 8 in the requested changes still stands – the links to the CITATIONS.bib file in README.md and in CONTRIBUTING.md will break as soon as this PR is merged into develop, so we should change the links here and let the link checker fail on that basis.

[prady0t]

@agriyakhetarpal RTD failure is now resolved.

[agriyakhetarpal]

I don't think "site-packages" would fix it – I triggered the workflows, but the editable installations are going to raise an error now.

[prady0t]

I see. Let me try something else

[prady0t]

We can do something like :

            if os.path.isfile(os.path.join(pybamm.root_dir(), "site-packages", "pybamm", "CITATIONS.bib")):
                citations_file = os.path.join(
                    pybamm.root_dir(), "site-packages", "pybamm", "CITATIONS.bib"
                )
            else:
                citations_file = os.path.join(pybamm.root_dir(), "src", "pybamm", "CITATIONS.bib")
                

Do you think it's correct? We can also use : https://stackoverflow.com/questions/122327/how-do-i-find-the-location-of-my-python-site-packages-directory#:~:text=python3%20%2Dc%20%27import%20sysconfig%3B%20print(sysconfig.get_paths()%5B%22purelib%22%5D)%27
to obtain file location.

[agriyakhetarpal]

How about avoiding pybamm.root_dir() altogether and just using pybamm.__path__[0]? That way, we can avoid an if-else condition.

[prady0t]

This should work.

[agriyakhetarpal]

Thanks, @prady0t, this LGTM now! As a note to other reviewers: the failing links are expected and will be resolved once this PR goes into develop.

[arjxn-py]

Thanks @prady0t, looks good

[agriyakhetarpal]

The example notebook looks like it is still failing: https://github.com/pybamm-team/PyBaMM/actions/runs/10300421066/job/28509802852?pr=4311

[prady0t]

Is it related to changes in this PR?

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[agriyakhetarpal]

Not sure what it was, but I removed the custom path because the PNGs are in the same directory as the notebook