Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change behavior of 'make debug' and 'make release' #7693

Closed
bangerth opened this issue Feb 5, 2019 · 13 comments
Closed

Change behavior of 'make debug' and 'make release' #7693

bangerth opened this issue Feb 5, 2019 · 13 comments

Comments

@bangerth
Copy link
Member

bangerth commented Feb 5, 2019

geodynamics/aspect#2800 is about an issue that has bugged me for a very long time already in ASPECT: When you call make debug, it switches to debug mode and re-compiles everything. This is fine if you have a tutorial with just one source file, but if you have a project with 263 source files this is annoying because cmake compiles everything with -j1. So it's exceedingly slow.

Might I suggest that we change the behavior of make debug and make release to just switch to the respective mode, delete the executable (to avoid anyone using it), and then asking the user to explicitly compile again?

(I'm apparently not the only one who has their own workaround to this issue -- @tjhei has as well: https://community.geodynamics.org/t/makefile-multiple-processor-issue/413?u=bangerth )

@kronbichler
Copy link
Member

You mean what happens in DEAL_II_INVOKE_AUTOPILOT? I would not be opposed as I have done so in some of my projects as well. On the other hand, from many other projects one expects that make debug does actually build some target and not just switches the build type, so the output needs to be clear (and what I see on the aspect list seems appropriate).

@tjhei
Copy link
Member

tjhei commented Feb 5, 2019

The main usage for the autopilot is for deal.II tutorial steps. As there is no parallel compilation needed (or possible), it is not a problem there, I think. So, maybe "don't use autopilot for big projects" is the simplest solution to this?

@bangerth
Copy link
Member Author

bangerth commented Feb 5, 2019

@tjhei: Yes, that's likely true. But I do think that we advertise the use of the autopilot in the sample "large project" cmake files.

@masterleinad
Copy link
Member

Why not extra targets like switch_to_release and switch_to_debug?

@drwells
Copy link
Member

drwells commented Feb 5, 2019

It might be possible to get around this with ninja and newer versions of CMake via the new (in 3.12) --parallel option:

-j [<jobs>], --parallel [<jobs>]
The maximum number of concurrent processes to use when building. If <jobs> is omitted the native build tool’s default number is used.

see

https://cmake.org/cmake/help/latest/manual/cmake.1.html

I don't think it is possible to get this to work in make, though; I haven't yet (see also #7143) been able to figure out a way to propagate -jN through make to cmake and back to make in this setting.

@tamiko
Copy link
Member

tamiko commented Feb 5, 2019

Well, the one responsible for this behavior in DEAL_II_INVOKE_AUTOPILOT is me cough. I think there was little thought that went into deciding this - originally this macro was only meant for really small projects (based on example steps).

That said, I don't see anything speaking against changing the behavior. The debug and release targets are probably overwhelmingly used in the following way:

make debug|release
make run

And here it doesn't really matter where the project is compiled.

@masterleinad
Copy link
Member

...unless you are using MPI. 😉

@tamiko
Copy link
Member

tamiko commented Feb 5, 2019

@masterleinad Huch? The depend line should ensure that the executable is always built prior to make run. Does this not work?

   93     ADD_CUSTOM_TARGET(run                                                        
   94       COMMAND ${_command}                                                        
   95       DEPENDS ${TARGET}                                                          
   96       COMMENT "Run ${TARGET} with ${CMAKE_BUILD_TYPE} configuration"             
   97       )

tamiko added a commit to tamiko/dealii that referenced this issue Feb 5, 2019
For a long time the "debug" and "release" targets of our convenience
macro DEAL_II_INVOKE_AUTOPILOT automatically rebuild the project when
switching to the debug or release flavor.

Closes dealii#7693
@masterleinad
Copy link
Member

No, I wanted to say that you usually don't invoke make run for applications that use MPI.

@tamiko
Copy link
Member

tamiko commented Feb 5, 2019

@masterleinad

SET(TARGET_RUN "mpirun -np 42 foobar")

and make run works 😆

@tjhei
Copy link
Member

tjhei commented Feb 5, 2019

In geodynamics/aspect#2800 I changed debug and release to not build but print a big comment about having to build. Maybe worth including here?

@tamiko
Copy link
Member

tamiko commented Feb 5, 2019

@tjhei I added your these messages to the pull request.

tamiko added a commit to tamiko/dealii that referenced this issue Feb 7, 2019
For a long time the "debug" and "release" targets of our convenience
macro DEAL_II_INVOKE_AUTOPILOT automatically rebuild the project when
switching to the debug or release flavor.

Closes dealii#7693
@bangerth
Copy link
Member Author

Nice, thanks @tamiko !

pratikvn pushed a commit to ginkgo-project/dealii that referenced this issue Apr 25, 2019
For a long time the "debug" and "release" targets of our convenience
macro DEAL_II_INVOKE_AUTOPILOT automatically rebuild the project when
switching to the debug or release flavor.

Closes dealii#7693
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

No branches or pull requests

6 participants